fix DSC Key Smuggling via Unsigned TBS Data [LA-A]#368
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR closes a security gap in the 1300-byte DSC verification circuit by ensuring the padded portion of the fixed-size TBS buffer is constrained to zero, preventing attacker-controlled trailing bytes from influencing downstream commitments while not being covered by the signature/hash path.
Changes:
- Import and apply
utils::check_zero_paddingtotbs_certificateint_add_dsc_verify_1300right after thetbs_certificate_lenbounds check. - Add an explanatory comment clarifying why the constraint is required for commitment safety.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…on, pubkey signed-length bound check added
dcbuild3r
pushed a commit
that referenced
this pull request
May 16, 2026
fix DSC Key Smuggling via Unsigned TBS Data [LA-A]
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
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.
Summary
This PR addresses the DSC Key Smuggling via Unsigned TBS Data audit finding. It includes:
1. Zero-padding constraint (1300-byte path)
check_zero_padding(tbs_certificate, tbs_certificate_len)int_add_dsc_verify_1300immediately after the length bounds checkutils::check_zero_padding, which already exists and is used in the 720-byte path2. Constrained ASN.1 length validation (both DSC circuits)
get_asn1_element_lengthconstrained utility toutils/utils/src/lib.nr— computes total TLV length from a DER element header withoutunconstrained/unsafet_add_dsc_720andt_add_dsc_verify_1300now validate the prover-suppliedtbs_certificate_lenagainst the value derived from the DER SEQUENCE header, preventing a malicious prover from lying about the signed length3. Pubkey signed-length bound check
verify_rsa_pubkey_in_tbsnow acceptstbs_certificate_lenand assertspubkey_offset + DSC_KEY_SIZE <= tbs_certificate_len, ensuring the key lies within the authenticated region of the TBS, not just within the fixed-size buffert_add_id_data_720andt_add_id_data_1300computetbs_certificate_lenfrom the DER header and pass it through4. DER SubjectPublicKeyInfo structural validation
verify_rsa_pubkey_in_tbsnow validates that the modulus is wrapped in a proper DER INTEGER (tag, long-form length, optional leading zero) and that the INTEGER is the first child of an RSAPublicKey SEQUENCE (30 82)dsc_pubkey, preventing a prover from manipulating which path is taken5. Modify ProverInputs and remove
signed_attributes_sizefieldpassport-input-genlibrary as they follow ASN.1 parsing.Background
The 1300-byte DSC registration circuit splits SHA-256 hashing across two sub-circuits (
t_add_dsc_hash_1300+t_add_dsc_verify_1300) using a pre-computed hash. Unlike the 720-byte path — which callssig_check_rsa::verify_signature→sha256_and_check_data_to_sign→check_zero_paddingautomatically — the 1300-byte path passes a pre-computedmsg_hash: [u8; 32]directly tofragmented_sig_check_rsa::verify_rsa_signature, which never sees the raw TBS buffer. This meant the zero-padding guard was never applied.Note
The following items from the audit are deferred to subsequent PRs:
Binding tbs_certificate_len into the commitment chain (if needed)
Audit suggestion 1 (separate PR)