Darwin mdns fix (#11992)

* Fix use after free in darwin DnssdImpl Also use safe string functions. * Use loopback for interface -1 on darwin. * Compile fixes. Disable adding the MDNS context to the list. Not sure what the correct solution is. * Remove the MDNS context handling, and free the text entries. * Restyled by clang-format * Use the existing method to clean up the context's memory. * Restyled by clang-format * Update src/platform/Darwin/DnssdImpl.cpp Co-authored-by: Michael Sandstedt <[email protected]> Co-authored-by: Andy Salisbury <[email protected]> Co-authored-by: Restyled.io <[email protected]> Co-authored-by: Michael Sandstedt <[email protected]>
project-chip · May 11, 2023 · 2662619 · 2662619
1 parent 564d46a
commit 2662619
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 7 deletions.
diff --git a/src/platform/Darwin/DnssdImpl.cpp b/src/platform/Darwin/DnssdImpl.cpp
@@ -89,7 +89,10 @@ void MdnsContexts::Delete(GenericContext * context)
         }
     }
 
-    DNSServiceRefDeallocate(context->serviceRef);
+    if (context->serviceRef != nullptr)
+    {
+        DNSServiceRefDeallocate(context->serviceRef);
+    }
     chip::Platform::Delete(context);
 }
 
@@ -460,13 +463,35 @@ static CHIP_ERROR GetAddrInfo(void * context, DnssdResolveCallback callback, uin
     protocol = kDNSServiceProtocol_IPv6;
 #endif
 
-    err = DNSServiceGetAddrInfo(&sdRef, 0 /* flags */, interfaceId, protocol, hostname, OnGetAddrInfo, sdCtx);
-    VerifyOrReturnError(CheckForSuccess(sdCtx, __func__, err, true), CHIP_ERROR_INTERNAL);
+    if (interfaceId != kDNSServiceInterfaceIndexLocalOnly)
+    {
+        // -1 is the local only interface. If we're not on that interface, we need to get the address for the given hostname.
+        err = DNSServiceGetAddrInfo(&sdRef, 0 /* flags */, interfaceId, protocol, hostname, OnGetAddrInfo, sdCtx);
+        VerifyOrReturnError(CheckForSuccess(sdCtx, __func__, err, true), CHIP_ERROR_INTERNAL);
 
-    err = DNSServiceSetDispatchQueue(sdRef, chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue());
-    VerifyOrReturnError(CheckForSuccess(sdCtx, __func__, err, true), CHIP_ERROR_INTERNAL);
+        err = DNSServiceSetDispatchQueue(sdRef, chip::DeviceLayer::PlatformMgrImpl().GetWorkQueue());
+        VerifyOrReturnError(CheckForSuccess(sdCtx, __func__, err, true), CHIP_ERROR_INTERNAL);
 
-    return MdnsContexts::GetInstance().Add(sdCtx, sdRef);
+        return MdnsContexts::GetInstance().Add(sdCtx, sdRef);
+    }
+    else
+    {
+        sockaddr_in6 sockaddr;
+        memset(&sockaddr, 0, sizeof(sockaddr));
+        sockaddr.sin6_len    = sizeof(sockaddr);
+        sockaddr.sin6_family = AF_INET6;
+        sockaddr.sin6_addr   = in6addr_loopback;
+        sockaddr.sin6_port   = htons((unsigned short) port);
+        uint32_t ttl         = 120; // default TTL for records with hostnames is 120 seconds
+        uint32_t interface   = 0;   // Set interface to ANY (0) - network stack can decide how to route this.
+        OnGetAddrInfo(nullptr, 0 /* flags */, interface, kDNSServiceErr_NoError, hostname,
+                      reinterpret_cast<struct sockaddr *>(&sockaddr), ttl, sdCtx);
+
+        // Don't leak memory.
+        sdCtx->serviceRef = nullptr;
+        MdnsContexts::GetInstance().Delete(sdCtx);
+        return CHIP_NO_ERROR;
+    }
 }
 
 static void OnResolve(DNSServiceRef sdRef, DNSServiceFlags flags, uint32_t interfaceId, DNSServiceErrorType err,

diff --git a/src/platform/Darwin/DnssdImpl.h b/src/platform/Darwin/DnssdImpl.h
@@ -125,12 +125,13 @@ class MdnsContexts
     void SetHostname(const char * name) { mHostname = name; }
     const char * GetHostname() { return mHostname.c_str(); }
 
+    void Delete(GenericContext * context);
+
 private:
     MdnsContexts(){};
     static MdnsContexts sInstance;
     std::string mHostname;
 
-    void Delete(GenericContext * context);
     std::vector<GenericContext *> mContexts;
 };