fix: double-free of BIGNUMs in _new_key_from_parameters()#140
Merged
atoomic merged 1 commit intoApr 3, 2026
Merged
Conversation
…parameters() Both the if (p||q) and else branches on OpenSSL 3.x free the BIGNUM parameters (n, e, d, p, q) after EVP_PKEY_fromdata() succeeds, since OSSL_PARAM_BLD_push_BN() copies the values. However, if make_rsa_obj() subsequently fails, execution falls through to the err: label which frees the same BIGNUMs again — a double-free. Fix: set pointers to NULL immediately after freeing so the error handler's BN_clear_free() calls become no-ops. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
timlegge
approved these changes
Mar 25, 2026
Member
timlegge
left a comment
timlegge
left a comment
There was a problem hiding this comment.
Good catch, Looks good to me.
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
3 participants
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.
What
NULL BIGNUM pointers after freeing them on the success path of
_new_key_from_parameters()to prevent double-free whenmake_rsa_obj()fails.Why
On OpenSSL 3.x, both the
if (p||q)andelsebranches free n, e, d (and p, q in the full-key branch) afterEVP_PKEY_fromdata()succeeds, sinceOSSL_PARAM_BLD_push_BN()copies the values. But ifmake_rsa_obj()subsequently fails (e.g., memory allocation failure), execution falls through to theerr:label which callsBN_clear_free()on the same pointers — a double-free.How
Set each pointer to NULL immediately after
BN_clear_free()on the success path, so the error handler's cleanup calls become no-ops. This follows the same NULL-after-free pattern already used forpctx,params_build,params, andctxin this function.Testing
Full test suite passes (507 tests). The double-free is only triggered under memory pressure (when
New()insidemake_rsa_objfails), so it wouldn't surface in normal testing — it's a latent bug visible through code inspection.🤖 Generated with Claude Code
Quality Report
Changes: 1 file changed, 2 insertions(+)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline