From 575579793766960ada9c6319206b61c5fac1c379 Mon Sep 17 00:00:00 2001
From: Abtin Keshavarzian <abtink@google.com>
Date: Tue, 26 Jul 2022 12:24:35 -0700
Subject: [PATCH] [routing-manager] fix `DiscoveredPrefixTable::Entry` leak
 (#7945)

This commit fixes an issue with the potential overwriting of `mNext`
when existing entry in the list is updated (avoid calling `Clear()`).
It also updates `test_routing_manager` to check such a situation
and ensure that the fix is addressing the issue.
---
 src/core/border_router/routing_manager.cpp |  3 --
 tests/unit/test_routing_manager.cpp        | 44 ++++++++++++++++++++++
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/src/core/border_router/routing_manager.cpp b/src/core/border_router/routing_manager.cpp
index 901945b20..dad8977d5 100644
--- a/src/core/border_router/routing_manager.cpp
+++ b/src/core/border_router/routing_manager.cpp
@@ -1861,7 +1861,6 @@ exit:
 
 void RoutingManager::DiscoveredPrefixTable::Entry::InitFrom(const Ip6::Nd::RouterAdvertMessage::Header &aRaHeader)
 {
-    Clear();
     mType                    = kTypeRoute;
     mValidLifetime           = aRaHeader.GetRouterLifetime();
     mShared.mRoutePreference = aRaHeader.GetDefaultRouterPreference();
@@ -1870,7 +1869,6 @@ void RoutingManager::DiscoveredPrefixTable::Entry::InitFrom(const Ip6::Nd::Route
 
 void RoutingManager::DiscoveredPrefixTable::Entry::InitFrom(const Ip6::Nd::PrefixInfoOption &aPio)
 {
-    Clear();
     aPio.GetPrefix(mPrefix);
     mType                      = kTypeOnLink;
     mValidLifetime             = aPio.GetValidLifetime();
@@ -1880,7 +1878,6 @@ void RoutingManager::DiscoveredPrefixTable::Entry::InitFrom(const Ip6::Nd::Prefi
 
 void RoutingManager::DiscoveredPrefixTable::Entry::InitFrom(const Ip6::Nd::RouteInfoOption &aRio)
 {
-    Clear();
     aRio.GetPrefix(mPrefix);
     mType                    = kTypeRoute;
     mValidLifetime           = aRio.GetRouteLifetime();
diff --git a/tests/unit/test_routing_manager.cpp b/tests/unit/test_routing_manager.cpp
index cf48e8119..56085d0d3 100644
--- a/tests/unit/test_routing_manager.cpp
+++ b/tests/unit/test_routing_manager.cpp
@@ -546,6 +546,50 @@ void TestRoutingManager(void)
 
     VerifyOrQuit(counter == 3);
 
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Send the same RA again from router A with the on-link (PIO) and route prefix (RIO).
+
+    {
+        Ip6::Nd::RouterAdvertMessage raMsg(Ip6::Nd::RouterAdvertMessage::Header(), buffer);
+
+        SuccessOrQuit(raMsg.AppendPrefixInfoOption(onLinkPrefix, kValidLitime, kPreferredLifetime));
+        SuccessOrQuit(raMsg.AppendRouteInfoOption(routePrefix, kValidLitime, NetworkData::kRoutePreferenceMedium));
+
+        SendRouterAdvert(routerAddressA, raMsg.GetAsPacket());
+
+        Log("Send RA from router A");
+        LogRouterAdvert(raMsg.GetAsPacket());
+    }
+
+    //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+    // Check the discovered prefix table and ensure info from router A
+    // remains unchanged.
+
+    counter = 0;
+
+    rm.InitPrefixTableIterator(iter);
+
+    while (rm.GetNextPrefixTableEntry(iter, entry) == kErrorNone)
+    {
+        counter++;
+        VerifyOrQuit(AsCoreType(&entry.mRouterAddress) == routerAddressA);
+
+        if (entry.mIsOnLink)
+        {
+            VerifyOrQuit(AsCoreType(&entry.mPrefix) == onLinkPrefix);
+            VerifyOrQuit(entry.mValidLifetime = kValidLitime);
+            VerifyOrQuit(entry.mPreferredLifetime = kPreferredLifetime);
+        }
+        else
+        {
+            VerifyOrQuit(AsCoreType(&entry.mPrefix) == routePrefix);
+            VerifyOrQuit(entry.mValidLifetime = kValidLitime);
+            VerifyOrQuit(static_cast<int8_t>(entry.mRoutePreference) == NetworkData::kRoutePreferenceMedium);
+        }
+    }
+
+    VerifyOrQuit(counter == 2);
+
     //- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
     // Send an RA from router B with same route prefix (RIO) but with
     // high route preference.