Skip to content

fix: sign() double buffer allocation and error-path leaks (OpenSSL 3.x)#80

Merged
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/fix-sign-error-leaks
Mar 14, 2026
Merged

fix: sign() double buffer allocation and error-path leaks (OpenSSL 3.x)#80
atoomic merged 1 commit into
cpan-authors:mainfrom
atoomic:koan.atoomic/fix-sign-error-leaks

Conversation

@Koan-Bot
Copy link
Copy Markdown
Contributor

Summary

Two issues in the OpenSSL 3.x signing path:

  1. Double buffer allocation: The signature buffer was allocated twice — first via CHECK_NEW (for EVP_PKEY_get_size bytes), then overwritten by Newx after querying the actual needed size. The first allocation leaked every time.

  2. Error-path resource leaks: If any OpenSSL call failed mid-setup, CHECK_OPEN_SSL would croak() immediately, leaking the EVP_PKEY_CTX and EVP_MD objects.

Fix

  • Removed the premature buffer allocation — the signature buffer is now allocated only once, after querying the actual needed size via EVP_PKEY_sign(ctx, NULL, ...)
  • Replaced CHECK_OPEN_SSL with THROW()/goto err pattern for proper cleanup
  • Moved EVP_PKEY_CTX and EVP_MD variable declarations to PREINIT: for function-wide scope

Test plan

  • All 278 existing tests pass
  • sign/verify tested in rsa.t with md5, sha1, sha224, sha256, sha384, sha512, ripemd160

🤖 Generated with Claude Code

Two issues in the OpenSSL 3.x signing path:

1. The signature buffer was allocated twice: first via CHECK_NEW for
   EVP_PKEY_get_size bytes, then overwritten by Newx after querying
   the actual size via EVP_PKEY_sign(NULL), leaking the first buffer.

2. If any OpenSSL call failed mid-setup, CHECK_OPEN_SSL would croak
   immediately, leaking the EVP_PKEY_CTX and EVP_MD objects.

Fix: use THROW/goto err pattern for proper cleanup, and allocate the
signature buffer only once after querying the needed size.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@atoomic atoomic marked this pull request as ready for review February 20, 2026 02:29
@atoomic atoomic requested a review from timlegge March 12, 2026 02:20
@timlegge timlegge self-assigned this Mar 13, 2026
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.

LGTM

@atoomic atoomic merged commit 66e2fe4 into cpan-authors:main Mar 14, 2026
50 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.

3 participants