[1.1] Cherry pick Thread DNS client and memory leak fixes (project-ch…

…ip#31457)

* [app] Fix DeferredAttributePersister memory leak (project-chip#31075)

* [app] Fix DeferredAttributePerister memory leak

ScopedMemoryBuffer's Release() method was used instead of
Free(). Add CHECK_RETURN_VALUE annotation to the Release()
method to prevent from making such a mistake in the future.

Signed-off-by: Damian Krolik <[email protected]>

* Code review

---------

Signed-off-by: Damian Krolik <[email protected]>
(cherry picked from commit 3e8aeeb)

* [OpenThread] Harden DNS record parsing (project-chip#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 <[email protected]>
(cherry picked from commit 76b6bb5)

src/app/DeferredAttributePersistenceProvider.cpp

@@ -39,7 +39,7 @@ void DeferredAttribute::Flush(AttributePersistenceProvider & persister)
 {
     VerifyOrReturn(IsArmed());
     persister.WriteValue(mPath, ByteSpan(mValue.Get(), mValue.AllocatedSize()));
-    mValue.Release();
+    mValue.Free();
 }
 
 CHIP_ERROR DeferredAttributePersistenceProvider::WriteValue(const ConcreteAttributePath & aPath, const ByteSpan & aValue)

src/darwin/Framework/CHIP/MTRBaseDevice.mm

@@ -1141,8 +1141,8 @@ - (void)readAttributePaths:(NSArray<MTRAttributeRequestPath *> * _Nullable)attri
             //
             callback->AdoptReadClient(std::move(readClient));
             callback.release();
-            attributePathParamsList.Release();
-            eventPathParamsList.Release();
+            IgnoreUnusedVariable(attributePathParamsList.Release());
+            IgnoreUnusedVariable(eventPathParamsList.Release());
             return err;
         });
     std::move(*bridge).DispatchAction(self);

src/lib/support/ScopedBuffer.h

@@ -25,6 +25,7 @@
 #pragma once
 
 #include <lib/support/CHIPMem.h>
+#include <lib/support/CodeUtils.h>
 
 #include <type_traits>
 #include <utility>
@@ -84,10 +85,11 @@ class ScopedMemoryBufferBase
     const void * Ptr() const { return mBuffer; }
 
     /**
-     * Releases the undelying buffer. Buffer stops being managed and will not be
-     * auto-freed.
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
      */
-    void * Release()
+    CHECK_RETURN_VALUE void * Release()
     {
         void * buffer = mBuffer;
         mBuffer       = nullptr;
@@ -139,13 +141,18 @@ class ScopedMemoryBuffer : public Impl::ScopedMemoryBufferBase<MemoryManagement>
 
     static_assert(std::is_trivially_destructible<T>::value, "Destructors won't get run");
 
-    inline T * Get() { return static_cast<T *>(Base::Ptr()); }
-    inline T & operator[](size_t index) { return Get()[index]; }
+    T * Get() { return static_cast<T *>(Base::Ptr()); }
+    T & operator[](size_t index) { return Get()[index]; }
 
-    inline const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
-    inline const T & operator[](size_t index) const { return Get()[index]; }
+    const T * Get() const { return static_cast<const T *>(Base::Ptr()); }
+    const T & operator[](size_t index) const { return Get()[index]; }
 
-    inline T * Release() { return static_cast<T *>(Base::Release()); }
+    /**
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
+     */
+    CHECK_RETURN_VALUE T * Release() { return static_cast<T *>(Base::Release()); }
 
     ScopedMemoryBuffer & Calloc(size_t elementCount)
     {
@@ -222,7 +229,12 @@ class ScopedMemoryBufferWithSize : public ScopedMemoryBuffer<T>
         ScopedMemoryBuffer<T>::Free();
     }
 
-    T * Release()
+    /**
+     * Releases the underlying buffer.
+     *
+     * The buffer stops being managed and will not be auto-freed.
+     */
+    CHECK_RETURN_VALUE T * Release()
     {
         T * buffer = ScopedMemoryBuffer<T>::Release();
         mCount     = 0;

src/platform/OpenThread/GenericThreadStackManagerImpl_OpenThread.hpp

@@ -94,6 +94,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
 
@@ -2317,29 +2340,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)
     {
@@ -2354,24 +2356,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.