Skip to content

Backport a few patches for v0.8.11 - #623

Open
stefanberger wants to merge 2 commits into
stable-0.8from
stable-0.8.next
Open

stefanberger wants to merge 2 commits into
stable-0.8from
stable-0.8.next

Conversation

@stefanberger

@stefanberger stefanberger commented Sep 4, 2026 •

Copy link
Copy Markdown
Owner

This PR backports a few patches for v0.8.11

Summary by CodeRabbit

  • Bug Fixes
    • Improved input handling during data parsing by rejecting negative buffer sizes.
    • Prevented potential out-of-bounds reads and invalid memory copies when processing insufficient or malformed data.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 4aa5f782-877b-4cb5-83dc-942a1dddc3f2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 8c8a33a5-df56-4565-991f-6bd1234177f7

📥 Commits

Reviewing files that changed from the base of the PR and between e99bb7e and 4ab38b7.

📒 Files selected for processing (1)
  • src/tpm2/Unmarshal.c

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

📝 Walkthrough

The primitive unmarshaling functions now reject negative remaining-buffer sizes with TPM_RC_INSUFFICIENT. This prevents invalid size conversions and memory-copy lengths.

Changes: Unmarshaling validation

Layer / File(s) Summary
Reject negative remaining-buffer sizes
src/tpm2/Unmarshal.c
UINT8_Unmarshal, UINT16_Unmarshal, UINT32_Unmarshal, UINT64_Unmarshal, and Array_Unmarshal return TPM_RC_INSUFFICIENT before reading or copying data when *size is negative.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 4ab38

TPM unmarshaling now rejects invalid negative buffer sizes before processing data, preventing unsafe length conversion and memory access. The change is ready to merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the primary change as backporting patches for the v0.8.11 release. It is concise and relevant, although it does not describe the negative-size validation details.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stable-0.8.next

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…rypt

TPM_SymmetricKeyData_Decrypt (AES path in src/tpm12/tpm_crypto.c,
OpenSSL backend; and the NSS/FreeBL backend in
src/tpm12/tpm_crypto_freebl.c) only checked
encrypt_length < TPM_AES_BLOCK_SIZE before calling
AES_cbc_encrypt(..., AES_DECRYPT) on a buffer allocated to exactly
encrypt_length bytes. CBC decryption always consumes a whole final
block, so a length that is not a multiple of 16 makes OpenSSL read up
to 15 bytes past the end of the input buffer.

That length is TPM_CONTEXT_BLOB.sensitiveData.size from the command
stream. TPM_Process_LoadKeyContext and TPM_Process_LoadAuthContext run
under TPM_CHECK_ALLOW_NO_OWNER and decrypt the blob with the permanent
contextKey before the integrity HMAC is checked, so the over-read is
reachable with a single unauthenticated command.

Reproduced with a TPM_LoadKeyContext command whose sensitiveData.size
is 17; AddressSanitizer reports a heap over-read:

    READ of size 4 ...
        #0 AES_decrypt aes_core.c
        #1 CRYPTO_cbc128_decrypt cbc128.c
        #2 TPM_SymmetricKeyData_Decrypt tpm_crypto.c
        #3 TPM_Process_LoadKeyContext tpm_session.c

The fix rejects a length that is not a whole number of blocks, the same
requirement the DES backend already enforces. The command then returns
TPM_DECRYPT_ERROR. A zero length is still rejected by TPM_Malloc().

Signed-off-by: sameera <sameera@research.itecz.au>
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.

2 participants