Fix BIGNUM leak in _new_key_from_parameters on pre-3.x#195
Draft
toddr-bot wants to merge 1 commit into
Draft
Conversation
On OpenSSL < 3.0, RSA_set0_key/RSA_set0_factors transfer ownership of BIGNUM pointers to the RSA struct. If a subsequent THROW fired between ownership transfer and the NULLing of those pointers, the err handler would unconditionally call BN_clear_free on already-transferred BIGNUMs, causing a double-free (RSA_free also frees them). Conversely, if THROW fired *before* ownership transfer, the err handler on pre-3.x had no code to free the pointer_copy'd BIGNUMs at all, leaking them. Fix both issues with an `xfer` bitmask that tracks which groups of BIGNUMs have been transferred to the RSA struct: bit 0 = n, e bit 1 = p, q bit 2 = d The err handler now conditionally frees only un-transferred BIGNUMs on pre-3.x. On 3.0+, OSSL_PARAM_BLD_push_BN copies values so the caller always owns the originals and the existing unconditional free is correct. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_new_key_from_parameterscould leak or double-free BIGNUMs when an error occurred between ownership transfer (RSA_set0_key/RSA_set0_factors/direct struct assign) and the NULLing of those pointersxferbitmask (bit 0=n+e, bit 1=p+q, bit 2=d) that tracks which BIGNUM groups have been transferred to the RSA structerrhandler now conditionally frees only un-transferred BIGNUMs on pre-3.x; 3.0+ path unchanged (OSSL_PARAM_BLD copies values, so caller always owns originals)Test plan
🤖 Generated with Claude Code