From 76b6bb5bcc13c4de6aa67885f6c2328fc8b5998b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Damian=20Kr=C3=B3lik?=
 <66667989+Damian-Nordic@users.noreply.github.com>
Date: Thu, 4 Jan 2024 00:47:27 +0100
Subject: [PATCH] [OpenThread] Harden DNS record parsing (#31227)

OpenThread applications would crash upon receiving an empty
DNS TXT record. The reason was that the code for copying OT
DNS service info object into Matter DnssdService object
would not initialize the TXT entry count in the latter
object in such a case.

In the reported case, the Matter stack was presented an
empty TXT record because OpenThread's DNS client received
a TXT record with TTL 0 and it discarded its contents.
Nevertheless, the issue could be reproduced by publishing
Matter service without TXT entries and kicking off DNS query.

1. Initialize the TXT entry and subtype count properly in all
   scenarios.
2. Do not even process the service info object if an error was
   returned by OpenThread before.
3. Extract some boilerplate to a separate function to improve
   readability.

Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
---
 ...nericThreadStackManagerImpl_OpenThread.hpp | 72 +++++++++----------
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp
index 4c4360822029cf..c1fd8635d33eb0 100644
--- a/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp
+++ b/src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp
@@ -91,6 +91,29 @@ void initNetworkCommissioningThreadDriver(void)
 #endif
 }
 
+#if CHIP_DEVICE_CONFIG_ENABLE_THREAD_DNS_CLIENT
+CHIP_ERROR ReadDomainNameComponent(const char *& in, char * out, size_t outSize)
+{
+    const char * dotPos = strchr(in, '.');
+    VerifyOrReturnError(dotPos != nullptr, CHIP_ERROR_INVALID_ARGUMENT);
+
+    const size_t componentSize = static_cast<size_t>(dotPos - in);
+    VerifyOrReturnError(componentSize < outSize, CHIP_ERROR_INVALID_ARGUMENT);
+
+    memcpy(out, in, componentSize);
+    out[componentSize] = '\0';
+    in += componentSize + 1;
+
+    return CHIP_NO_ERROR;
+}
+
+template <size_t N>
+CHIP_ERROR ReadDomainNameComponent(const char *& in, char (&out)[N])
+{
+    return ReadDomainNameComponent(in, out, N);
+}
+#endif
+
 NetworkCommissioning::otScanResponseIterator<NetworkCommissioning::ThreadScanResponse> mScanResponseIter;
 } // namespace
 
@@ -2320,29 +2343,8 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::FromOtDnsRespons
 {
     char protocol[chip::Dnssd::kDnssdProtocolTextMaxSize + 1];
 
-    if (strchr(serviceType, '.') == nullptr)
-        return CHIP_ERROR_INVALID_ARGUMENT;
-
-    // Extract from the <type>.<protocol>.<domain-name>. the <type> part.
-    size_t substringSize = strchr(serviceType, '.') - serviceType;
-    if (substringSize >= ArraySize(mdnsService.mType))
-    {
-        return CHIP_ERROR_INVALID_ARGUMENT;
-    }
-    Platform::CopyString(mdnsService.mType, substringSize + 1, serviceType);
-
-    // Extract from the <type>.<protocol>.<domain-name>. the <protocol> part.
-    const char * protocolSubstringStart = serviceType + substringSize + 1;
-
-    if (strchr(protocolSubstringStart, '.') == nullptr)
-        return CHIP_ERROR_INVALID_ARGUMENT;
-
-    substringSize = strchr(protocolSubstringStart, '.') - protocolSubstringStart;
-    if (substringSize >= ArraySize(protocol))
-    {
-        return CHIP_ERROR_INVALID_ARGUMENT;
-    }
-    Platform::CopyString(protocol, substringSize + 1, protocolSubstringStart);
+    ReturnErrorOnFailure(ReadDomainNameComponent(serviceType, mdnsService.mType));
+    ReturnErrorOnFailure(ReadDomainNameComponent(serviceType, protocol));
 
     if (strncmp(protocol, "_udp", chip::Dnssd::kDnssdProtocolTextMaxSize) == 0)
     {
@@ -2357,24 +2359,20 @@ CHIP_ERROR GenericThreadStackManagerImpl_OpenThread<ImplClass>::FromOtDnsRespons
         mdnsService.mProtocol = chip::Dnssd::DnssdServiceProtocol::kDnssdProtocolUnknown;
     }
 
+    mdnsService.mInterface     = Inet::InterfaceId::Null();
+    mdnsService.mSubTypeSize   = 0;
+    mdnsService.mTextEntrySize = 0;
+
     // Check if SRV record was included in DNS response.
-    if (error != OT_ERROR_NOT_FOUND)
+    // If not, return partial information about the service and exit early.
+    if (error != OT_ERROR_NONE)
     {
-        if (strchr(serviceInfo.mHostNameBuffer, '.') == nullptr)
-            return CHIP_ERROR_INVALID_ARGUMENT;
-
-        // Extract from the <hostname>.<domain-name>. the <hostname> part.
-        substringSize = strchr(serviceInfo.mHostNameBuffer, '.') - serviceInfo.mHostNameBuffer;
-        if (substringSize >= ArraySize(mdnsService.mHostName))
-        {
-            return CHIP_ERROR_INVALID_ARGUMENT;
-        }
-        Platform::CopyString(mdnsService.mHostName, substringSize + 1, serviceInfo.mHostNameBuffer);
-
-        mdnsService.mPort = serviceInfo.mPort;
+        return CHIP_NO_ERROR;
     }
 
-    mdnsService.mInterface = Inet::InterfaceId::Null();
+    const char * host = serviceInfo.mHostNameBuffer;
+    ReturnErrorOnFailure(ReadDomainNameComponent(host, mdnsService.mHostName));
+    mdnsService.mPort = serviceInfo.mPort;
 
     // Check if AAAA record was included in DNS response.