diff --git a/src/CertManager.sol b/src/CertManager.sol index 29ecfd0..c1e2dda 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -382,7 +382,11 @@ contract CertManager is ICertManager { revert InvalidCertAlgorithm(); } // as extensions are used in cert, version should be 3 (value 2) as per https://datatracker.ietf.org/doc/html/rfc5280#section-4.1.2.1 - if (certificate.uintAt(certificate.firstChildOf(versionPtr)) != 2) revert InvalidCertVersion(); + Asn1Ptr versionValuePtr = certificate.firstChildOf(versionPtr); + if (versionValuePtr.content() + versionValuePtr.length() != versionPtr.content() + versionPtr.length()) { + revert Asn1Decode.InvalidAsn1Length(); + } + if (certificate.uintAt(versionValuePtr) != 2) revert InvalidCertVersion(); (notAfter, maxPathLen, issuerHash, subjectHash, pubKey) = _parseTbsInner(certificate, sigAlgoPtr, ca, ptr.content() + ptr.length()); diff --git a/test/CertManager.t.sol b/test/CertManager.t.sol index 6db1293..5bcd42f 100644 --- a/test/CertManager.t.sol +++ b/test/CertManager.t.sol @@ -7,6 +7,7 @@ import {ICertManager} from "../src/ICertManager.sol"; import {Asn1Decode, LibAsn1Ptr, Asn1Ptr} from "../src/Asn1Decode.sol"; import {LibBytes} from "../src/LibBytes.sol"; import {P384Verifier} from "../src/P384Verifier.sol"; +import {IP384Verifier} from "../src/IP384Verifier.sol"; import {P384HintCollector} from "./helpers/HintedNitroTestHelpers.sol"; contract Asn1DecodeHarness { @@ -52,6 +53,19 @@ contract CertManagerExtensionsHarness is CertManager { } } +contract RevertingP384Verifier is IP384Verifier { + error SignatureVerificationCalled(); + + function verifyP384SignatureWithHints(bytes memory, bytes memory, bytes memory, bytes memory) + external + pure + override + returns (bool) + { + revert SignatureVerificationCalled(); + } +} + contract CertManagerTest is Test { using Asn1Decode for bytes; using LibAsn1Ptr for Asn1Ptr; @@ -166,6 +180,15 @@ contract CertManagerTest is Test { cm.verifyCACertWithHints(mutated, keccak256(CB0), ""); } + function test_ParseTbsRejectsTrailingVersionWrapperFieldBeforeSignatureVerification() public { + vm.warp(1775145600); + CertManager cm = new CertManager(new RevertingP384Verifier()); + bytes memory mutated = _appendVersionTrailingField(CB1); + + vm.expectRevert(Asn1Decode.InvalidAsn1Length.selector); + cm.verifyCACertWithHints(mutated, keccak256(CB0), ""); + } + function test_ParsePubKeyAcceptsUncompressedP384Point() public view { bytes memory pubKey = _patternBytes(96); bytes memory spki = abi.encodePacked(hex"3076301006072a8648ce3d020106052b8104002203620004", pubKey); @@ -419,6 +442,19 @@ contract CertManagerTest is Test { _writeDerLength(result, tbsPtr, _addDelta(tbsPtr.length(), delta)); } + function _appendVersionTrailingField(bytes memory certificate) internal pure returns (bytes memory result) { + Asn1Ptr root = certificate.root(); + Asn1Ptr tbsPtr = certificate.firstChildOf(root); + Asn1Ptr versionPtr = certificate.firstChildOf(tbsPtr); + bytes memory nullField = hex"0500"; + int256 delta = int256(nullField.length); + result = _insertBytes(certificate, versionPtr.content() + versionPtr.length(), nullField); + + _writeDerLength(result, root, _addDelta(root.length(), delta)); + _writeDerLength(result, tbsPtr, _addDelta(tbsPtr.length(), delta)); + _writeDerLength(result, versionPtr, _addDelta(versionPtr.length(), delta)); + } + function _appendSignatureTrailingField(bytes memory certificate) internal pure returns (bytes memory result) { (Asn1Ptr root, Asn1Ptr sigPtr, Asn1Ptr sigRoot,) = _certSignaturePtrs(certificate); bytes memory extraInteger = hex"020100";