Skip to content

Commit

Permalink
Fix use-after-free in CommissioningWindowOpener. (project-chip#22767)
Browse files Browse the repository at this point in the history
Once we call back into our client, it can delete us, so we need to do
any logging that uses `mSetupPayload` before we do that.

Fixes project-chip#22765
bzbarsky-apple authored and andy31415 committed Sep 23, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
1 parent 989ad8e commit 5059157
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/controller/CommissioningWindowOpener.cpp
Original file line number Diff line number Diff line change
@@ -204,9 +204,6 @@ void CommissioningWindowOpener::OnOpenCommissioningWindowSuccess(void * context,
self->mNextStep = Step::kAcceptCommissioningStart;
if (self->mCommissioningWindowCallback != nullptr)
{
self->mCommissioningWindowCallback->mCall(self->mCommissioningWindowCallback->mContext, self->mNodeId, CHIP_NO_ERROR,
self->mSetupPayload);

char payloadBuffer[QRCodeBasicSetupPayloadGenerator::kMaxQRCodeBase38RepresentationLength + 1];

MutableCharSpan manualCode(payloadBuffer);
@@ -230,11 +227,18 @@ void CommissioningWindowOpener::OnOpenCommissioningWindowSuccess(void * context,
{
ChipLogError(Controller, "Unable to generate QR code for setup payload: %" CHIP_ERROR_FORMAT, err.Format());
}

self->mCommissioningWindowCallback->mCall(self->mCommissioningWindowCallback->mContext, self->mNodeId, CHIP_NO_ERROR,
self->mSetupPayload);
// Don't touch `self` anymore; it might have been destroyed by the
// callee.
}
else if (self->mBasicCommissioningWindowCallback != nullptr)
{
self->mBasicCommissioningWindowCallback->mCall(self->mBasicCommissioningWindowCallback->mContext, self->mNodeId,
CHIP_NO_ERROR);
// Don't touch `self` anymore; it might have been destroyed by the
// callee.
}
}

0 comments on commit 5059157

Please sign in to comment.