From 387589687a70087f52db8719c3b10671e8398f40 Mon Sep 17 00:00:00 2001
From: Jonathan Hui <jonhui@google.com>
Date: Sat, 22 Aug 2026 15:44:01 -0700
Subject: [PATCH] [mle] validate router ID bounds in router-id lookups (#13539)

Two router-id lookups indexed their backing array by a Router ID
from a received message before checking it against `kMaxRouterId`:

1. `RouterIdMask::IsAllocated(aRouterId)` read `mMask[aRouterId / 8]`
   (an 8-byte array). When `HandleAddressSolicitResponse()` passed
   `GetLeaderId()`, the Leader Router ID copied verbatim from a
   received Leader Data TLV, router IDs 64-255 read up to 24 bytes
   past `mMask`.
2. `RouterTable::RouterIdMap::IsAllocated(aRouterId)` read
   `mIndexes[aRouterId]` (a 63-byte array for IDs 0..62). When an MLE
   Advertisement had a Source Address decoding to router ID 63 (e.g.
   `0xFC00`), `RoleTransitioner::DecideWhetherToDowngrade()` checked
   allocation of the neighbor ID and read 1 byte past `mIndexes`.

This commit updates both `RouterIdMask::IsAllocated()` and
`RouterTable::RouterIdMap::IsAllocated()` to verify that the router
ID is within `[0, kMaxRouterId]` before indexing the array. It also
adds unit tests in `test_mle.cpp` validating bounds handling.
---
 src/core/thread/mle_types.hpp    |  5 ++-
 src/core/thread/router_table.hpp |  5 ++-
 tests/unit/test_mle.cpp          | 53 ++++++++++++++++++++++++++++++++
 3 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/src/core/thread/mle_types.hpp b/src/core/thread/mle_types.hpp
index 24a86a61e..4b9f76089 100644
--- a/src/core/thread/mle_types.hpp
+++ b/src/core/thread/mle_types.hpp
@@ -666,7 +666,10 @@ public:
      * @retval TRUE   If the Router ID bit is set in the mask.
      * @retval FALSE  If the Router ID bit is not set in the mask.
      */
-    bool IsAllocated(uint8_t aRouterId) const { return (mMask[aRouterId / 8] & MaskFor(aRouterId)) != 0; }
+    bool IsAllocated(uint8_t aRouterId) const
+    {
+        return (aRouterId <= kMaxRouterId) && ((mMask[aRouterId / 8] & MaskFor(aRouterId)) != 0);
+    }
 
     /**
      * Sets a given Router ID in the mask.
diff --git a/src/core/thread/router_table.hpp b/src/core/thread/router_table.hpp
index 824c92f6d..e8c3b8860 100644
--- a/src/core/thread/router_table.hpp
+++ b/src/core/thread/router_table.hpp
@@ -465,7 +465,10 @@ private:
         // remaining reuse delay time (in seconds).
 
         RouterIdMap(void) { Clear(); }
-        bool    IsAllocated(uint8_t aRouterId) const { return (mIndexes[aRouterId] & kAllocatedFlag); }
+        bool IsAllocated(uint8_t aRouterId) const
+        {
+            return (aRouterId <= Mle::kMaxRouterId) && ((mIndexes[aRouterId] & kAllocatedFlag) != 0);
+        }
         uint8_t GetIndex(uint8_t aRouterId) const { return (mIndexes[aRouterId] & kIndexMask); }
         void    SetIndex(uint8_t aRouterId, uint8_t aIndex) { mIndexes[aRouterId] = kAllocatedFlag | aIndex; }
         bool    CanAllocate(uint8_t aRouterId) const { return (mIndexes[aRouterId] == 0); }
diff --git a/tests/unit/test_mle.cpp b/tests/unit/test_mle.cpp
index 186c35a36..3e4f60935 100644
--- a/tests/unit/test_mle.cpp
+++ b/tests/unit/test_mle.cpp
@@ -37,6 +37,7 @@
 #include "thread/mle_tlvs.hpp"
 #include "thread/mle_types.hpp"
 #include "thread/network_data_leader.hpp"
+#include "thread/router_table.hpp"
 
 namespace ot {
 
@@ -849,15 +850,67 @@ void TestLeaderWeightCalculation(void)
 
 #endif // #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_MLE_DEVICE_PROPERTY_LEADER_WEIGHT_ENABLE
 
+void TestRouterIdMask(void)
+{
+    Mle::RouterIdMask mask;
+
+    mask.Clear();
+    VerifyOrQuit(mask.IsValid());
+    VerifyOrQuit(mask.DetermineAllocatedCount() == 0);
+
+    for (uint16_t routerId = 0; routerId <= 255; routerId++)
+    {
+        VerifyOrQuit(!mask.IsAllocated(static_cast<uint8_t>(routerId)));
+    }
+
+    mask.Add(0);
+    mask.Add(10);
+    mask.Add(Mle::kMaxRouterId);
+
+    VerifyOrQuit(mask.IsAllocated(0));
+    VerifyOrQuit(mask.IsAllocated(10));
+    VerifyOrQuit(mask.IsAllocated(Mle::kMaxRouterId));
+    VerifyOrQuit(!mask.IsAllocated(1));
+    VerifyOrQuit(!mask.IsAllocated(61));
+
+    for (uint16_t routerId = Mle::kMaxRouterId + 1; routerId <= 255; routerId++)
+    {
+        VerifyOrQuit(!mask.IsAllocated(static_cast<uint8_t>(routerId)));
+    }
+
+    mask.Remove(10);
+    VerifyOrQuit(!mask.IsAllocated(10));
+
+    printf("TestRouterIdMask passed\n");
+}
+
+#if OPENTHREAD_FTD
+void TestRouterTableRouterIdBounds(void)
+{
+    Instance    *instance    = static_cast<Instance *>(testInitInstance());
+    RouterTable &routerTable = instance->Get<RouterTable>();
+
+    for (uint16_t routerId = 0; routerId <= 255; routerId++)
+    {
+        VerifyOrQuit(!routerTable.IsAllocated(static_cast<uint8_t>(routerId)));
+    }
+
+    testFreeInstance(instance);
+    printf("TestRouterTableRouterIdBounds passed\n");
+}
+#endif
+
 } // namespace ot
 
 int main(void)
 {
     ot::TestDeviceMode();
+    ot::TestRouterIdMask();
     ot::UnitTester::TestChildIdResponseNetworkDataHandling();
 
 #if OPENTHREAD_FTD
     ot::UnitTester::TestTxChallengeTable();
+    ot::TestRouterTableRouterIdBounds();
 #endif
 
 #if OPENTHREAD_FTD && OPENTHREAD_CONFIG_MLE_DEVICE_PROPERTY_LEADER_WEIGHT_ENABLE