Fix infinite recusion possibility in QName parsing (#18394)

project-chip · Oct 11, 2023 · 7341860 · 7341860
1 parent 62c872e
commit 7341860
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 0 deletions.
diff --git a/src/lib/dnssd/minimal_mdns/core/QName.cpp b/src/lib/dnssd/minimal_mdns/core/QName.cpp
@@ -74,6 +74,14 @@ bool SerializedQNameIterator::Next(bool followIndirectPointers)
                 return false;
             }
 
+            // Look behind has to keep going backwards, otherwise we may
+            // get into an infinite list
+            if (offset >= static_cast<size_t>(mCurrentPosition - mValidData.Start()))
+            {
+                mIsValid = false;
+                return false;
+            }
+
             mLookBehindMax   = offset;
             mCurrentPosition = mValidData.Start() + offset;
         }

diff --git a/src/lib/dnssd/minimal_mdns/core/tests/TestQName.cpp b/src/lib/dnssd/minimal_mdns/core/tests/TestQName.cpp
@@ -120,7 +120,10 @@ void ErrorTest(nlTestSuite * inSuite, void * inContext)
         NL_TEST_ASSERT(inSuite, !it.Next());
         NL_TEST_ASSERT(inSuite, !it.IsValid());
     }
+}
 
+void InvalidReferencing(nlTestSuite * inSuite, void * inContext)
+{
     {
         // Truncated before the end (but seemingly valid in case of error)
         // does NOT use AsSerializedQName (because out of range)
@@ -130,6 +133,7 @@ void ErrorTest(nlTestSuite * inSuite, void * inContext)
         NL_TEST_ASSERT(inSuite, !it.Next());
         NL_TEST_ASSERT(inSuite, !it.IsValid());
     }
+
     {
         // Infinite recursion
         static const uint8_t kData[] = "\03test\xc0\x00";
@@ -139,6 +143,35 @@ void ErrorTest(nlTestSuite * inSuite, void * inContext)
         NL_TEST_ASSERT(inSuite, !it.Next());
         NL_TEST_ASSERT(inSuite, !it.IsValid());
     }
+
+    {
+        // Infinite recursion by referencing own element (inside the stream)
+        static const uint8_t kData[] = "\03test\xc0\x05";
+        SerializedQNameIterator it   = AsSerializedQName(kData);
+
+        NL_TEST_ASSERT(inSuite, it.Next());
+        NL_TEST_ASSERT(inSuite, !it.Next());
+        NL_TEST_ASSERT(inSuite, !it.IsValid());
+    }
+
+    {
+        // Infinite recursion by referencing own element at the start
+        static const uint8_t kData[] = "\xc0\x00";
+        SerializedQNameIterator it   = AsSerializedQName(kData);
+
+        NL_TEST_ASSERT(inSuite, !it.Next());
+        NL_TEST_ASSERT(inSuite, !it.IsValid());
+    }
+
+    {
+        // Reference that goes forwad instead of backward
+        static const uint8_t kData[] = "\03test\xc0\x07";
+        SerializedQNameIterator it   = AsSerializedQName(kData);
+
+        NL_TEST_ASSERT(inSuite, it.Next());
+        NL_TEST_ASSERT(inSuite, !it.Next());
+        NL_TEST_ASSERT(inSuite, !it.IsValid());
+    }
 }
 
 void Comparison(nlTestSuite * inSuite, void * inContext)
@@ -279,6 +312,7 @@ static const nlTest sTests[] =
     NL_TEST_DEF("CaseInsensitiveSerializedCompare", CaseInsensitiveSerializedCompare),
     NL_TEST_DEF("CaseInsensitiveFullQNameCompare", CaseInsensitiveFullQNameCompare),
     NL_TEST_DEF("SerializedCompare", SerializedCompare),
+    NL_TEST_DEF("InvalidReferencing", InvalidReferencing),
 
     NL_TEST_SENTINEL()
 };