Skip to content

Fix uninitialized EVP_MD_CTX and harden bn_dup_into#3033

Merged
justsmth merged 2 commits intoaws:mainfrom
justsmth:rsa-md_ctx-error-path
Mar 6, 2026
Merged

Fix uninitialized EVP_MD_CTX and harden bn_dup_into#3033
justsmth merged 2 commits intoaws:mainfrom
justsmth:rsa-md_ctx-error-path

Conversation

@justsmth
Copy link
Contributor

Description of changes:

In rsa_key_fips_pairwise_consistency_test_signing, md_ctx was used uninitialized if EVP_PKEY_new or EVP_PKEY_set1_RSA failed before EVP_MD_CTX_init was reached. The cleanup path would then call EVP_MD_CTX_cleanse on garbage stack data. Move the init to immediately after declaration so all goto paths are safe.

Also add an aliasing guard to bn_dup_into to prevent a use-after-free if *dst and src ever point to the same BIGNUM, and remove a dead variable left over from refactoring in RSA_check_fips.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.37%. Comparing base (e2b4850) to head (bc39a06).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crypto/fipsmodule/rsa/rsa.c 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3033   +/-   ##
=======================================
  Coverage   78.37%   78.37%           
=======================================
  Files         689      689           
  Lines      121148   121148           
  Branches    16973    16974    +1     
=======================================
  Hits        94948    94948           
- Misses      25302    25303    +1     
+ Partials      898      897    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@justsmth justsmth requested review from dkostic and geedo0 February 25, 2026 13:54
@justsmth justsmth merged commit 0130e24 into aws:main Mar 6, 2026
442 of 455 checks passed
@justsmth justsmth deleted the rsa-md_ctx-error-path branch March 6, 2026 21:02
WillChilds-Klein pushed a commit to WillChilds-Klein/aws-lc that referenced this pull request Mar 11, 2026
### Description of changes: 
In `rsa_key_fips_pairwise_consistency_test_signing`, `md_ctx` was used
uninitialized if `EVP_PKEY_new` or `EVP_PKEY_set1_RSA` failed before
`EVP_MD_CTX_init` was reached. The cleanup path would then call
`EVP_MD_CTX_cleanse` on garbage stack data. Move the init to immediately
after declaration so all goto paths are safe.

Also add an aliasing guard to `bn_dup_into` to prevent a use-after-free
if `*dst` and `src` ever point to the same `BIGNUM`, and remove a dead
variable left over from refactoring in `RSA_check_fips`.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.

Co-authored-by: Sanketh Menda <sgmenda@amazon.com>
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.

4 participants