fix: reject non-canonical ASN.1 lengths

Author: leanthebean

src/Asn1Decode.sol

@@ -231,6 +231,7 @@ library Asn1Decode {
     }
 
     function readNodeLength(bytes memory der, uint256 ix) private pure returns (Asn1Ptr) {
+        require(ix + 1 < der.length, "truncated ASN.1 header");
         require(der[ix] & 0x1f != 0x1f, "ASN.1 tags longer than 1-byte are not supported");
         uint256 length;
         uint256 ixFirstContentByte;
@@ -239,6 +240,10 @@ library Asn1Decode {
             ixFirstContentByte = ix + 2;
         } else {
             uint8 lengthbytesLength = uint8(der[ix + 1] & 0x7F);
+            require(lengthbytesLength != 0, "indefinite ASN.1 length");
+            require(lengthbytesLength <= 32, "ASN.1 length too long");
+            require(ix + 2 + lengthbytesLength <= der.length, "truncated ASN.1 length");
+            require(der[ix + 2] != 0, "non-canonical ASN.1 length");
             if (lengthbytesLength == 1) {
                 length = uint8(der[ix + 2]);
             } else if (lengthbytesLength == 2) {
@@ -247,8 +252,10 @@ library Asn1Decode {
                 length = uint256(readBytesN(der, ix + 2, lengthbytesLength) >> (32 - lengthbytesLength) * 8);
                 require(length <= 2 ** 64 - 1); // bound to max uint64 to be safe
             }
+            require(length >= 128, "non-canonical ASN.1 length");
             ixFirstContentByte = ix + 2 + lengthbytesLength;
         }
+        require(ixFirstContentByte + length <= der.length, "ASN.1 node length exceeds input");
         return LibAsn1Ptr.toAsn1Ptr(ix, ixFirstContentByte, length);
     }
 

test/Asn1Decode.t.sol

@@ -62,6 +62,28 @@ contract Asn1DecodeTest is Test {
         h.rootLength(hex"0289ffffffffffffffffff"); // INTEGER, 9 length bytes all 0xff
     }
 
+    function test_root_indefiniteLength_reverts() public {
+        vm.expectRevert("indefinite ASN.1 length");
+        h.rootLength(hex"0480"); // DER requires definite lengths
+    }
+
+    function test_root_longFormForShortLength_reverts() public {
+        vm.expectRevert("non-canonical ASN.1 length");
+        h.rootLength(hex"04810100"); // length 1 must use short form 0x01
+    }
+
+    function test_root_longFormLeadingZero_reverts() public {
+        vm.expectRevert("non-canonical ASN.1 length");
+        h.rootLength(hex"04820080"); // length 128 must be 0x81 0x80, not 0x82 0x00 0x80
+    }
+
+    function test_root_canonicalLongFormLength() public view {
+        bytes memory der = abi.encodePacked(bytes3(0x048180), new bytes(128));
+
+        assertEq(h.rootLength(der), 128);
+        assertEq(h.rootContent(der), 3);
+    }
+
     // --- uintAt ---
 
     function test_uintAt_value() public view {

test/hinted/HintedNitroAttestation.t.sol

@@ -478,6 +478,19 @@ contract HintedNitroAttestationTest is Test {
         certManager.verifyCACertWithHints(abi.encodePacked(caCert, bytes1(0x00)), parentHash, "");
     }
 
+    function test_HintedCACertRejectsNonCanonicalOuterLength() public {
+        bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64()));
+        (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation);
+        NitroValidator.Ptrs memory ptrs = parser.parseAttestation(attestationTbs);
+        (bytes memory caCert, bytes32 parentHash,) = _firstNonRootCA(attestationTbs, ptrs);
+        bytes memory nonCanonicalCert = _nonCanonicalOuterSequenceLength(caCert);
+        bytes32 nonCanonicalHash = keccak256(nonCanonicalCert);
+
+        vm.expectRevert("non-canonical ASN.1 length");
+        certManager.verifyCACertWithHints(nonCanonicalCert, parentHash, "");
+        assertEq(certManager.loadVerified(nonCanonicalHash).pubKey.length, 0, "non-canonical cert must not cache");
+    }
+
     function test_HintedTrailingRootBytesCannotPoisonParentCache() public {
         bytes memory attestation = _repairMissingPublicKeyBytes(_decodeBase64(_realAttestationB64()));
         (bytes memory attestationTbs,) = validator.decodeAttestationTbs(attestation);
@@ -1255,6 +1268,21 @@ contract HintedNitroAttestationTest is Test {
         output[3] = bytes1(uint8(length));
     }
 
+    function _nonCanonicalOuterSequenceLength(bytes memory der) internal pure returns (bytes memory output) {
+        require(der.length >= 4 && der[0] == 0x30 && der[1] == 0x82, "test: expected long sequence");
+
+        output = new bytes(der.length + 1);
+        output[0] = der[0];
+        output[1] = 0x83;
+        output[2] = 0x00;
+        output[3] = der[2];
+        output[4] = der[3];
+
+        for (uint256 i = 4; i < der.length; ++i) {
+            output[i + 1] = der[i];
+        }
+    }
+
     function _repairMissingPublicKeyBytes(bytes memory attestation) internal pure returns (bytes memory repaired) {
         // The pasted Base64 sample is missing "ic_" in the CBOR key
         // "public_key", but the key length and outer COSE payload length still