-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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.
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
Leak in Darwin protocol for publicKey #32301
Conversation
e7eaa14
to
96e78ba
Compare
PR #32301: Size comparison from 22bb737 to 89da639 Full report (41 builds for bl602, bl702, bl702l, cc13x4_26x4, esp32, k32w, mbed, qpg, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the issue a memory leak, or a leak of the key material? The PR description does not say clearly.....
publicKey = [keypair copyPublicKey]; | ||
} else { | ||
publicKey = [keypair publicKey]; | ||
if (publicKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curlies around body, please.
CHIP_ERROR err = MTRP256KeypairBridge::MatterPubKeyFromSecKeyRef(params.nocSigner.publicKey, &pubKey); | ||
SecKeyRef publicKey = NULL; | ||
|
||
if ([params.nocSigner respondsToSelector:@selector(copyPublicKey)]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a utility function for this, or better yet a utility RAII helper, instead of copy/pasting this code around.
PR #32301: Size comparison from 0840ca6 to 557535b Full report (32 builds for efr32, linux, mbed, nrfconnect, nxp, telink, tizen)
|
This PR has not been updated in a long time. Assuming stale and closing. |
* Restyled by whitespace * Restyled by clang-format --------- Co-authored-by: Restyled.io <commits@restyled.io>
PR #32301: Size comparison from 7df7676 to 17336cd Full report (68 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
The protocol we're asking folks to implement basically guarantees a key leak, this should be an explicit copy.