From 76d787ead52a37345866ad3e40ecb4ff239a13f8 Mon Sep 17 00:00:00 2001 From: Leopold Joy Date: Wed, 17 Jun 2026 01:07:15 +0100 Subject: [PATCH] Harden attestation payload parsing Co-authored-by: OpenCode --- src/NitroValidator.sol | 10 ++++++++-- test/IndefiniteLengthCbor.t.sol | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/NitroValidator.sol b/src/NitroValidator.sol index 06c1a6e..8073aee 100644 --- a/src/NitroValidator.sol +++ b/src/NitroValidator.sol @@ -212,11 +212,17 @@ contract NitroValidator { // be present (do not silently stop at the payload end on a missing break), and stop // exclusively on it. require(current.end() < end, "missing break marker"); - if (uint8(attestationTbs[current.end()]) == 0xff) break; + if (uint8(attestationTbs[current.end()]) == 0xff) { + require(current.end() + 1 == end, "trailing payload bytes"); + break; + } } else { // A definite-length map ends after exactly `entryCount` entries; a stray 0xFF must // not terminate it early (it would be parsed as a key and rejected as a non-string). - if (entry == entryCount) break; + if (entry == entryCount) { + require(current.end() == end, "trailing payload bytes"); + break; + } } current = attestationTbs.nextTextString(current); bytes32 keyHash = attestationTbs.keccak(current); diff --git a/test/IndefiniteLengthCbor.t.sol b/test/IndefiniteLengthCbor.t.sol index a74b531..2838352 100644 --- a/test/IndefiniteLengthCbor.t.sol +++ b/test/IndefiniteLengthCbor.t.sol @@ -835,6 +835,22 @@ contract NitroValidatorIndefiniteLengthTest is Test { validator.parseAttestation(tbs); } + /// @dev Definite-length outer maps must consume the whole payload byte string; trailing bytes + /// after the declared entries are malformed, even if the declared prefix parses cleanly. + function test_neg_definiteOuterMapTrailingBytes_reverts() public { + bytes memory tbs = _buildTbs(abi.encodePacked(hex"a2", _partialEntries(), hex"00")); + vm.expectRevert("trailing payload bytes"); + validator.parseAttestation(tbs); + } + + /// @dev Indefinite-length outer maps must end immediately after their 0xFF break marker; bytes + /// after the break are not silently ignored. + function test_neg_indefiniteOuterMapTrailingBytesAfterBreak_reverts() public { + bytes memory tbs = _buildTbs(abi.encodePacked(CBOR_MAP_INDEFINITE, _partialEntries(), CBOR_BREAK, hex"00")); + vm.expectRevert("trailing payload bytes"); + validator.parseAttestation(tbs); + } + /// @dev Empty indefinite-length inner cabundle array ([0x9F, 0xFF]) parses as an /// empty cabundle and the outer loop continues. function test_nestedIndefiniteEmptyArray_parses() public view {