Skip to content

fix: coordinate RETVAL/error guards in _new_key_from_parameters()#160

Merged
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/fix-issue-156
Apr 3, 2026
Merged

fix: coordinate RETVAL/error guards in _new_key_from_parameters()#160
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/fix-issue-156

Conversation

@Koan-Bot
Copy link
Copy Markdown
Contributor

@Koan-Bot Koan-Bot commented Apr 3, 2026

Summary

_new_key_from_parameters() called make_rsa_obj() and jumped to end: on success via if(RETVAL) goto end. On the failure path (falsy return without croak), error remained 0, so the if(error) cleanup block skipped EVP_PKEY_free(rsa), leaking the key. Replace the pattern with THROW(RETVAL = make_rsa_obj(...)) + goto end to ensure error = 1 is set before jumping to err: on any falsy return.

Fixes #156

Changes

  • Replace if(RETVAL) goto end with THROW(RETVAL = make_rsa_obj(...)) + goto end in _new_key_from_parameters() (RSA.xs)

Test plan

  • All 477 existing tests pass (make test)
  • The failure path (NULL return from make_rsa_obj without croaking) is not reachable today since CHECK_NEW always croaks — no new test added; fix is a robustness/control-flow correctness improvement

Generated by Kōan /fix


Quality Report

Changes: 1 file changed, 2 insertions(+), 3 deletions(-)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

Replace `if(RETVAL) goto end` with `THROW(RETVAL = make_rsa_obj(...))` so
that if make_rsa_obj ever returns NULL without croaking, error=1 is set
before jumping to err:, ensuring rsa is freed in the cleanup block.

The if(error) guard at err: was not coordinated with the if(RETVAL) guard,
leaving rsa leaked on a falsy-but-non-croak return. Not reachable today
since CHECK_NEW inside make_rsa_obj always croaks, but the control-flow
contract was fragile.

Fixes cpan-authors#156

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@timlegge timlegge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense

@timlegge timlegge marked this pull request as ready for review April 3, 2026 16:26
@atoomic atoomic merged commit 5039db3 into cpan-authors:main Apr 3, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

robustness: rsa leaks in _new_key_from_parameters() when make_rsa_obj returns falsy without setting error

3 participants