Skip to content

Check BN_new() and RSA_new() return values in generate_key()#184

Open
toddr-bot wants to merge 1 commit into
cpan-authors:mainfrom
toddr-bot:koan.toddr.bot/fix-generate-key-bn-null
Open

Check BN_new() and RSA_new() return values in generate_key()#184
toddr-bot wants to merge 1 commit into
cpan-authors:mainfrom
toddr-bot:koan.toddr.bot/fix-generate-key-bn-null

Conversation

@toddr-bot
Copy link
Copy Markdown
Contributor

Summary

  • Add NULL check after BN_new() — prevents segfault on memory exhaustion
  • Add NULL check after RSA_new() in the 0.9.8–2.x code path with proper cleanup

Why

BN_new() returns NULL on allocation failure. Without a check,
BN_set_word(NULL, exponent) dereferences a null pointer — a segfault
instead of a clean croak. Similarly, in the 0.9.8–2.x path,
RSA_new() failure would pass NULL to RSA_generate_key_ex().

Every other allocation in RSA.xs is checked (CHECK_OPEN_SSL, THROW,
or explicit if). These two were the last unchecked allocations in
generate_key().

Testing

  • Full test suite passes (all 16 test files)
  • Cannot easily unit-test malloc failure, but the fix is
    straightforward: add checks that match the existing patterns

🤖 Generated with Claude Code

BN_new() can return NULL on memory exhaustion. The subsequent
BN_set_word(e, exponent) would dereference NULL — a segfault instead
of a clean error. Similarly, on OpenSSL 0.9.8–2.x, RSA_new() was
unchecked; failure would pass NULL to RSA_generate_key_ex().

Add CHECK_OPEN_SSL(e) after BN_new(), and an explicit NULL check
with cleanup for RSA_new() in the 0.9.8–2.x code path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@timlegge timlegge marked this pull request as ready for review May 28, 2026 02:44
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.

looks good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants