Repository navigation
tpm2: fix EVP_PKEY/EVP_PKEY_CTX leak in CryptRsaEncrypt - #652
Conversation
When the runtime profile requires NO_UNPADDED_ENCRYPTION (no-unpadded-encryption or fips-host), the TPM_ALG_NULL case returned TPM_RC_SCHEME directly after pkey and ctx were already allocated, skipping the Exit label that frees them. Each TPM2_RSA_Encrypt with scheme TPM_ALG_NULL then leaks an EVP_PKEY and an EVP_PKEY_CTX. Use ERROR_EXIT like the matching guard in CryptRsaDecrypt. Signed-off-by: sameera <sameera@research.itecz.au>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Raw RSA error path src/tpm2/TPMCmd/tpm/src/crypt/CryptRsa.c |
When the runtime profile disallows unpadded encryption for TPM_ALG_NULL, the function now uses ERROR_EXIT(TPM_RC_SCHEME) instead of returning directly. |
Priority:
Estimated code review effort: 2 (Simple) | ~5 minutes
Change: Bug fix
Suggested reviewers: stefanberger
Merge Risk: 
30ece
-
No actionable merge-blocking risk remains in the reviewed change.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the primary change: fixing an EVP_PKEY/EVP_PKEY_CTX leak in CryptRsaEncrypt. The raw summary says the TPM_ALG_NULL guard now uses ERROR_EXIT instead of returning TPM_RC_SCHEME directly… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the primary change: fixing an EVP_PKEY/EVP_PKEY_CTX leak in CryptRsaEncrypt. The raw summary says the TPM_ALG_NULL guard now uses ERROR_EXIT instead of returning TPM_RC_SCHEME directly, so the error path reaches the existing cleanup. The title matches this change and the PR objectives.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
Thanks! |
the no-unpadded-encryption / fips-host guard in the TPM_ALG_NULL case returns TPM_RC_SCHEME directly after pkey and ctx are already allocated, so it skips the Exit: label and leaks an EVP_PKEY + EVP_PKEY_CTX on every TPM2_RSA_Encrypt(scheme=NULL); switched it to ERROR_EXIT like the same guard in CryptRsaDecrypt.
verified with a small harness that drives the real CryptRsaEncrypt under a no-unpadded-encryption profile and counts live OpenSSL allocations: ~11 leaked per call before, 0 after, same TPM_RC_SCHEME return either way. make check stays green (the fuzz state round-trip corpus included).
Summary by CodeRabbit