Skip to content

Commit

Permalink
Fix ResolverProxy use-after-free in HandleNodeBrowse (#13291)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
msandstedt authored Jan 4, 2022
1 parent ffdbd1e commit d4d57f6
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
4 changes: 4 additions & 0 deletions src/lib/core/ReferenceCounted.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<count_type>::max())
{
abort();
Expand Down
2 changes: 1 addition & 1 deletion src/lib/dnssd/Discovery_ImplPlatform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<ResolverDelegateProxy *>(context);
proxy->Release();

for (size_t i = 0; i < servicesSize; ++i)
{
Expand All @@ -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)
Expand Down

0 comments on commit d4d57f6

Please sign in to comment.