fix: uint384At misreads DER integers shorter than 48 bytes#25
Merged
Conversation
A P-384 ECDSA signature component (r or s) whose canonical 48-byte
big-endian representation starts with 0x00 is DER-encoded with fewer
than 48 bytes. After stripping the DER sign-padding byte, uint384At
was left with valueLength < 48. It applied a compensating right-shift
only to the lo word, leaving hi reading one byte too far into the value:
Buggy: hi = readBytesN(start, 16) >> 128 -- absorbs byte 15 into hi
lo = readBytesN(start+16, vl-16) >> shift -- shifts lo right, drops last byte
Fixed: shift = 48 - valueLength
hi = readBytesN(start, 16-shift) >> (128 + shift*8)
lo = readBytesN(start + 16-shift, 32)
The corrupted scalar is passed to ECDSA384.verify, which correctly
rejects it, causing CertManager._verifyCertSignature to revert with
"invalid sig". This makes CertManager.verifyCACert non-deterministically
fail for ~1/256 of all P-384 signature components (~1/128 per cert,
~3% per full attestation chain).
Regression tests use the live cabundle[3] from a 2026-04-02 Base Sepolia
dev attestation that reproducibly triggered the on-chain revert.
Made-with: Cursor
leopoldjoy
approved these changes
Apr 7, 2026
|
Good catch. Nice! |
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
4 participants
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
uint384AtinAsn1Decode.solincorrectly parses P-384 ECDSA signature components when their DER encoding is shorter than 48 bytes. This causesCertManager._verifyCertSignatureto receive a corrupted scalar,ECDSA384.verifyto returnfalse, and the transaction to revert with"invalid sig".Root cause
A P-384 ECDSA scalar is a 48-byte big-endian integer. When the scalar's most-significant byte is
0x00, DER omits it, encoding only 47 bytes.uint384Atstrips any DER sign-padding byte and then splits the remaining bytes into(hi: uint128, lo: uint256). The split must treat the value as zero-padded to exactly 48 bytes. The unfixed code does not do this forhi:Unfixed (valueLength = 47, shift = 1):
Fixed:
For the concrete cert that triggered this (
CB3in the regression test), thescomponent's DER-encoded value was 47 bytes. The unfixed code produced:hi0xcaf59019bfbcc6f6ed365e5a892ceaa20x00caf59019bfbcc6f6ed365e5a892cealo[0]0x00…(0xa2 absorbed into hi)0xa2…Probability
The bug fires whenever the most-significant byte of an ECDSA scalar is exactly
0x00. For a uniformly distributed 48-byte scalar this occurs with probability 1/256 ≈ 0.4% per scalar. Each cert signature has two scalars (rands); a typical Nitro attestation chain contains 4 certs — 8 scalars total:Processing a meaningful volume of attestations should likely encounter this revert eventually.
Reproduction
Both tests fail on unpatched code and pass after this fix. The cert data embedded in the tests is the exact chain from the 2026-04-02 ~15:35 UTC dev enclave attestation that produced the live revert.