fix(tdx): reject declared lengths that overrun the quote buffer - #250
Merged
Conversation
Four lengths in the signature section come from the quote, which is untrusted input: the signature-data size, cert_size, qe_auth_size and pck_size. Python slicing clamps instead of overreading, so an inflated length yielded a short slice and parsing continued against whatever fit. No read was ever out of bounds and the downstream signature verification would fail, so this is fail-closed hardening rather than a memory-safety fix. Still worth rejecting: appraising 300 bytes where the producer declared 400 means verifying something other than what they said they signed, and reporting that as a signature failure hides the real cause. Found while reviewing the same parse in cmcp#420, which shares this derivation and has the same gap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while reviewing cmcp#420, which derives the same parse independently and has the same gap. I offered the fix there, so here it is on the shared side.
The gap
Four lengths in the TDX signature section are declared by the quote, which is untrusted input: the signature-data size,
cert_size,qe_auth_size, andpck_size. Each was used to slice without checking it fits.Python slicing clamps rather than overreading, so an inflated length produced a short slice and parsing continued against whatever happened to fit. Nothing read out of bounds, and the downstream signature check would fail, so this is fail-closed hardening rather than a memory-safety fix.
It is still worth rejecting. Appraising 300 bytes where the producer declared 400 means verifying something other than what they said they signed, and surfacing that as a generic signature failure hides the actual cause from whoever has to debug it. The new errors say which length was wrong and how much was available.
Tests
Three, each tampering one declared length in an otherwise valid quote built by the suite's own helper:
cert_sizeqe_auth_size626 passed, 18 skipped. mypy, ruff, bandit clean.
pck_sizegets the same check but no separate test: with the preceding checks in place it is unreachable from a single-field tamper, and a test that cannot fail before the fix is not worth having.