Skip to content

Commit 5e1584b

Browse files
leopoldjoyOpenCode
andauthored
Harden attestation payload parsing (#34)
Co-authored-by: OpenCode <opencode-noreply@coinbase.com>
1 parent 331bb42 commit 5e1584b

2 files changed

Lines changed: 24 additions & 2 deletions

File tree

src/NitroValidator.sol

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,11 +212,17 @@ contract NitroValidator {
212212
// be present (do not silently stop at the payload end on a missing break), and stop
213213
// exclusively on it.
214214
require(current.end() < end, "missing break marker");
215-
if (uint8(attestationTbs[current.end()]) == 0xff) break;
215+
if (uint8(attestationTbs[current.end()]) == 0xff) {
216+
require(current.end() + 1 == end, "trailing payload bytes");
217+
break;
218+
}
216219
} else {
217220
// A definite-length map ends after exactly `entryCount` entries; a stray 0xFF must
218221
// not terminate it early (it would be parsed as a key and rejected as a non-string).
219-
if (entry == entryCount) break;
222+
if (entry == entryCount) {
223+
require(current.end() == end, "trailing payload bytes");
224+
break;
225+
}
220226
}
221227
current = attestationTbs.nextTextString(current);
222228
bytes32 keyHash = attestationTbs.keccak(current);

test/IndefiniteLengthCbor.t.sol

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -835,6 +835,22 @@ contract NitroValidatorIndefiniteLengthTest is Test {
835835
validator.parseAttestation(tbs);
836836
}
837837

838+
/// @dev Definite-length outer maps must consume the whole payload byte string; trailing bytes
839+
/// after the declared entries are malformed, even if the declared prefix parses cleanly.
840+
function test_neg_definiteOuterMapTrailingBytes_reverts() public {
841+
bytes memory tbs = _buildTbs(abi.encodePacked(hex"a2", _partialEntries(), hex"00"));
842+
vm.expectRevert("trailing payload bytes");
843+
validator.parseAttestation(tbs);
844+
}
845+
846+
/// @dev Indefinite-length outer maps must end immediately after their 0xFF break marker; bytes
847+
/// after the break are not silently ignored.
848+
function test_neg_indefiniteOuterMapTrailingBytesAfterBreak_reverts() public {
849+
bytes memory tbs = _buildTbs(abi.encodePacked(CBOR_MAP_INDEFINITE, _partialEntries(), CBOR_BREAK, hex"00"));
850+
vm.expectRevert("trailing payload bytes");
851+
validator.parseAttestation(tbs);
852+
}
853+
838854
/// @dev Empty indefinite-length inner cabundle array ([0x9F, 0xFF]) parses as an
839855
/// empty cabundle and the outer loop continues.
840856
function test_nestedIndefiniteEmptyArray_parses() public view {

0 commit comments

Comments
 (0)