From c50811c1d355261664150550310a2596c970117b Mon Sep 17 00:00:00 2001 From: Michael Sandstedt Date: Thu, 30 Dec 2021 08:15:05 -0600 Subject: [PATCH] Fix ResolverProxy use-after-free in HandleNodeBrowse HandleNodeBrowse decrements the ResolverProxy reference count, which will cause the object to be destructed if the counter reaches 0. It then increments the counter and accesses the object, which can be a use-after-free. This commit fixes the problem by ordering Release to occur after Retain. This commit also adds an abort to ReferenceCounted::Retain to check for cases like this when kInitRefCount is non-zero. For objects that are initialized with a non-zero reference count, we don't ever expect to call Retain when the count has already decremented to 0 because this indicates the object has been deleted. Fixes #13289 --- src/lib/core/ReferenceCounted.h | 4 ++++ src/lib/dnssd/Discovery_ImplPlatform.cpp | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/lib/core/ReferenceCounted.h b/src/lib/core/ReferenceCounted.h index 52ebc56d13a7e3..3086c902180a19 100644 --- a/src/lib/core/ReferenceCounted.h +++ b/src/lib/core/ReferenceCounted.h @@ -51,6 +51,10 @@ class ReferenceCounted /** Adds one to the usage count of this class */ Subclass * Retain() { + if (kInitRefCount && mRefCount == 0) + { + abort(); + } if (mRefCount == std::numeric_limits::max()) { abort(); diff --git a/src/lib/dnssd/Discovery_ImplPlatform.cpp b/src/lib/dnssd/Discovery_ImplPlatform.cpp index 8822774648407c..ca32481b5ca8b9 100644 --- a/src/lib/dnssd/Discovery_ImplPlatform.cpp +++ b/src/lib/dnssd/Discovery_ImplPlatform.cpp @@ -129,7 +129,6 @@ static void HandleNodeIdResolve(void * context, DnssdService * result, CHIP_ERRO static void HandleNodeBrowse(void * context, DnssdService * services, size_t servicesSize, CHIP_ERROR error) { ResolverDelegateProxy * proxy = static_cast(context); - proxy->Release(); for (size_t i = 0; i < servicesSize; ++i) { @@ -144,6 +143,7 @@ static void HandleNodeBrowse(void * context, DnssdService * services, size_t ser HandleNodeResolve(context, &services[i], error); } } + proxy->Release(); } CHIP_ERROR AddPtrRecord(DiscoveryFilter filter, const char ** entries, size_t & entriesCount, char * buffer, size_t bufferLen)