Skip to content

Commit

Permalink
Fix use-after-free in CommissioningWindowOpener. (#22767) (#22849)
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 #22765

Co-authored-by: Boris Zbarsky <[email protected]>
  • Loading branch information
andy31415 and bzbarsky-apple authored Sep 23, 2022
1 parent 062f85c commit 5f29159
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
Expand Up @@ -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);
Expand All @@ -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.
}
}

Expand Down

0 comments on commit 5f29159

Please sign in to comment.