From 2679c39ef18f165d71596d2df78d925476364a68 Mon Sep 17 00:00:00 2001 From: Elena Nadolinski Date: Thu, 18 Jun 2026 15:07:15 -0700 Subject: [PATCH] fix: bound attestation CBOR container counts --- src/NitroValidator.sol | 10 ++++++++-- test/IndefiniteLengthCbor.t.sol | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/NitroValidator.sol b/src/NitroValidator.sol index 8073aee..5e3c131 100644 --- a/src/NitroValidator.sol +++ b/src/NitroValidator.sol @@ -27,6 +27,9 @@ contract NitroValidator { bytes32 public constant NONCE_KEY = 0x7ab1577440dd7bedf920cb6de2f9fc6bf7ba98c78c85a3fa1f8311aac95e1759; // keccak256(bytes("nonce")) bytes32 public constant PCRS_KEY = 0x61585f8bc67a4b6d5891a4639a074964ac66fc2241dc0b36c157dc101325367a; // keccak256(bytes("pcrs")) + uint256 public constant MAX_PCRS = 32; + uint256 public constant MAX_CABUNDLE_CERTS = 32; + struct Ptrs { CborElement moduleID; uint64 timestamp; @@ -113,7 +116,7 @@ contract NitroValidator { require(ptrs.timestamp > 0, "no timestamp"); require(ptrs.cabundle.length > 0, "no cabundle"); require(attestationTbs.keccak(ptrs.digest) == ATTESTATION_DIGEST, "invalid digest"); - require(1 <= ptrs.pcrs.length && ptrs.pcrs.length <= 32, "invalid pcrs"); + require(1 <= ptrs.pcrs.length && ptrs.pcrs.length <= MAX_PCRS, "invalid pcrs"); require( ptrs.publicKey.isNull() || (1 <= ptrs.publicKey.length() && ptrs.publicKey.length() <= 1024), "invalid pub key" @@ -194,7 +197,8 @@ contract NitroValidator { /// AWS's COSE signature, so unknown content is signed and ignoring it cannot change the /// accept decision. /// - The outer payload map and the nested `pcrs` map / `cabundle` array are each accepted in - /// both definite-length and indefinite-length CBOR form. + /// both definite-length and indefinite-length CBOR form. Parser-time count caps are applied + /// before allocation, so malformed pre-auth payloads cannot force unbounded memory growth. function _parseAttestation(bytes memory attestationTbs) internal pure returns (Ptrs memory) { require(attestationTbs.keccak(0, 18) == ATTESTATION_TBS_PREFIX, "invalid attestation prefix"); @@ -278,6 +282,7 @@ contract NitroValidator { current = tbs.nextArray(keyPtr); bool indefinite = _isIndefinite(tbs, headerIx); uint256 count = indefinite ? _countIndefiniteItems(tbs, current.end()) : current.value(); + require(count <= MAX_CABUNDLE_CERTS, "too many cabundle certs"); cabundle = new CborElement[](count); for (uint256 i = 0; i < count; i++) { current = tbs.nextByteString(current); @@ -308,6 +313,7 @@ contract NitroValidator { } else { count = current.value(); } + require(count <= MAX_PCRS, "too many pcrs"); pcrs = new CborElement[](count); for (uint256 i = 0; i < count; i++) { current = tbs.nextPositiveInt(current); diff --git a/test/IndefiniteLengthCbor.t.sol b/test/IndefiniteLengthCbor.t.sol index 2838352..d3f9f4e 100644 --- a/test/IndefiniteLengthCbor.t.sol +++ b/test/IndefiniteLengthCbor.t.sol @@ -783,6 +783,36 @@ contract NitroValidatorIndefiniteLengthTest is Test { _assertSyntheticFields(validator.parseAttestation(tbs)); } + /// @dev Malicious definite-length cabundle count must be bounded before allocating the pointer + /// array. This is the 34-byte payload shape from the gas-grief finding. + function test_neg_largeCabundleCount_revertsBeforeAllocation() public { + bytes memory tbs = _buildTbs( + abi.encodePacked( + hex"a1", // one top-level map entry + hex"68636162756e646c65", // key "cabundle" + hex"9a000fffff" // array(1,048,575) + ) + ); + assertEq(tbs.length, 34, "expected minimal large-cabundle payload"); + + vm.expectRevert("too many cabundle certs"); + validator.parseAttestation(tbs); + } + + /// @dev Malicious definite-length pcrs count must be bounded before allocating the pointer array. + function test_neg_largePcrsCount_revertsBeforeAllocation() public { + bytes memory tbs = _buildTbs( + abi.encodePacked( + hex"a1", // one top-level map entry + hex"6470637273", // key "pcrs" + hex"ba000fffff" // map(1,048,575) + ) + ); + + vm.expectRevert("too many pcrs"); + validator.parseAttestation(tbs); + } + /// @dev Indefinite-length map without a trailing 0xFF break marker. /// Parser reads past valid entries into trailing garbage, reverting /// when it encounters a non-text-string byte as the next key.