Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[BUG] Use-after-free in CommissioningWindowOpener #22765

Closed
bzbarsky-apple opened this issue Sep 20, 2022 · 0 comments · Fixed by #22767
Closed

[BUG] Use-after-free in CommissioningWindowOpener #22765

bzbarsky-apple opened this issue Sep 20, 2022 · 0 comments · Fixed by #22767

Comments

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Sep 20, 2022 •
edited
Loading

Reproduction steps

CommissioningWindowOpener::OnOpenCommissioningWindowSuccess does this:

    if (self->mCommissioningWindowCallback != nullptr)
    {
        self->mCommissioningWindowCallback->mCall(self->mCommissioningWindowCallback->mContext, self->mNodeId, CHIP_NO_ERROR,
                                                  self->mSetupPayload);
...        CHIP_ERROR err = ManualSetupPayloadGenerator(self->mSetupPayload).payloadDecimalStringRepresentation(manualCode);

but the call to mCall above can (and usually does) delete self. So the self->mSetupPayload use is use-after-free.

What I can't figure out is why neither TSAN nor ASAN is catching it....

Bug prevalence

Any time ECM windows are opened via CommisioningWindowOpener

GitHub hash of the SDK that was being used

685c4d5

Platform

core

Platform Version(s)

No response

Anything else?

No response
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue Sep 20, 2022
@bzbarsky-apple
Fix use-after-free in CommissioningWindowOpener.
1409eef 
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 bzbarsky-apple mentioned this issue Sep 20, 2022
Merged
andy31415 pushed a commit that referenced this issue Sep 21, 2022
@bzbarsky-apple
Fix use-after-free in CommissioningWindowOpener. (#22767)
9bb2f53 
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
andy31415 pushed a commit to andy31415/connectedhomeip that referenced this issue Sep 23, 2022
@bzbarsky-apple @andy31415
Fix use-after-free in CommissioningWindowOpener. (project-chip#22767)
5059157 
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
@andy31415 andy31415 mentioned this issue Sep 23, 2022
Merged
andy31415 added a commit that referenced this issue Sep 23, 2022
@andy31415 @bzbarsky-apple
Fix use-after-free in CommissioningWindowOpener. (#22767) (#22849)
5f29159 
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix use-after-free in CommissioningWindowOpener. bzbarsky-apple/connectedhomeip
1 participant
@bzbarsky-apple