From cb5a4b669440d8f81fe98d4b245b836e0b8c283a Mon Sep 17 00:00:00 2001 From: Leopold Joy Date: Fri, 26 Jun 2026 12:43:38 +0100 Subject: [PATCH 1/2] fix: require exact cert ASN.1 tags Co-authored-by: OpenCode --- src/CertManager.sol | 27 ++++++++++++++++++++++++++- test/CertManager.t.sol | 29 +++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/CertManager.sol b/src/CertManager.sol index d1d9515..b80614c 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -17,6 +17,8 @@ contract CertManager is ICertManager { using LibAsn1Ptr for Asn1Ptr; using LibBytes for bytes; + error InvalidAsn1Tag(); + event CertVerified(bytes32 indexed certHash); event CertRevoked(bytes32 indexed certHash); event CertUnrevoked(bytes32 indexed certHash); @@ -163,7 +165,10 @@ contract CertManager is ICertManager { /// {revokeCerts}; the same value is recorded on-chain when the cert is first verified, so a /// revocation applies to every byte-encoding of that certificate. Reverts on malformed DER. function computeCertId(bytes memory cert) external pure returns (bytes32) { - Asn1Ptr tbsCertPtr = cert.firstChildOf(cert.root()); + Asn1Ptr root = cert.root(); + _requireAsn1Tag(cert, root, 0x30); + Asn1Ptr tbsCertPtr = cert.firstChildOf(root); + _requireAsn1Tag(cert, tbsCertPtr, 0x30); return _certIdentity(cert, tbsCertPtr); } @@ -290,11 +295,17 @@ contract CertManager is ICertManager { } Asn1Ptr root = certificate.root(); + _requireAsn1Tag(certificate, root, 0x30); require(root.totalLength() == certificate.length, "invalid cert length"); Asn1Ptr tbsCertPtr = certificate.firstChildOf(root); + _requireAsn1Tag(certificate, tbsCertPtr, 0x30); return certificate.keccak(tbsCertPtr.header(), tbsCertPtr.totalLength()); } + function _requireAsn1Tag(bytes memory der, Asn1Ptr ptr, bytes1 tag) internal pure { + if (der[ptr.header()] != tag) revert InvalidAsn1Tag(); + } + function _isPinnedRootAlias(bytes32 certHash, VerifiedCert memory cert) internal pure returns (bool) { return certHash != ROOT_CA_CERT_HASH && cert.ca && cert.subjectHash == ROOT_CA_CERT_SUBJECT_HASH && cert.pubKey.length == ROOT_CA_CERT_PUB_KEY.length @@ -308,8 +319,10 @@ contract CertManager is ICertManager { bytes memory signatureHints ) internal view returns (VerifiedCert memory cert, bytes32 identity) { Asn1Ptr root = certificate.root(); + _requireAsn1Tag(certificate, root, 0x30); require(root.totalLength() == certificate.length, "invalid cert length"); Asn1Ptr tbsCertPtr = certificate.firstChildOf(root); + _requireAsn1Tag(certificate, tbsCertPtr, 0x30); (uint64 notAfter, int64 maxPathLen, bytes32 issuerHash, bytes32 subjectHash, bytes memory pubKey) = _parseTbs(certificate, tbsCertPtr, ca); @@ -349,10 +362,13 @@ contract CertManager is ICertManager { view returns (uint64 notAfter, int64 maxPathLen, bytes32 issuerHash, bytes32 subjectHash, bytes memory pubKey) { + _requireAsn1Tag(certificate, ptr, 0x30); Asn1Ptr versionPtr = certificate.firstChildOf(ptr); + _requireAsn1Tag(certificate, versionPtr, 0xa0); Asn1Ptr vPtr = certificate.firstChildOf(versionPtr); Asn1Ptr serialPtr = certificate.nextSiblingOf(versionPtr); Asn1Ptr sigAlgoPtr = certificate.nextSiblingOf(serialPtr); + _requireAsn1Tag(certificate, sigAlgoPtr, 0x30); require(certificate.keccak(sigAlgoPtr.content(), sigAlgoPtr.length()) == CERT_ALGO_OID, "invalid cert sig algo"); uint256 version = certificate.uintAt(vPtr); @@ -368,11 +384,15 @@ contract CertManager is ICertManager { returns (uint64 notAfter, int64 maxPathLen, bytes32 issuerHash, bytes32 subjectHash, bytes memory pubKey) { Asn1Ptr issuerPtr = certificate.nextSiblingOf(sigAlgoPtr); + _requireAsn1Tag(certificate, issuerPtr, 0x30); issuerHash = certificate.keccak(issuerPtr.content(), issuerPtr.length()); Asn1Ptr validityPtr = certificate.nextSiblingOf(issuerPtr); + _requireAsn1Tag(certificate, validityPtr, 0x30); Asn1Ptr subjectPtr = certificate.nextSiblingOf(validityPtr); + _requireAsn1Tag(certificate, subjectPtr, 0x30); subjectHash = certificate.keccak(subjectPtr.content(), subjectPtr.length()); Asn1Ptr subjectPublicKeyInfoPtr = certificate.nextSiblingOf(subjectPtr); + _requireAsn1Tag(certificate, subjectPublicKeyInfoPtr, 0x30); Asn1Ptr extensionsPtr = certificate.nextSiblingOf(subjectPublicKeyInfoPtr); if (certificate[extensionsPtr.header()] == 0x81) { @@ -394,7 +414,9 @@ contract CertManager is ICertManager { pure returns (bytes memory subjectPubKey) { + _requireAsn1Tag(certificate, subjectPublicKeyInfoPtr, 0x30); Asn1Ptr pubKeyAlgoPtr = certificate.firstChildOf(subjectPublicKeyInfoPtr); + _requireAsn1Tag(certificate, pubKeyAlgoPtr, 0x30); Asn1Ptr pubKeyAlgoIdPtr = certificate.firstChildOf(pubKeyAlgoPtr); Asn1Ptr algoParamsPtr = certificate.nextSiblingOf(pubKeyAlgoIdPtr); Asn1Ptr subjectPublicKeyPtr = certificate.nextSiblingOf(pubKeyAlgoPtr); @@ -435,6 +457,7 @@ contract CertManager is ICertManager { { require(certificate[extensionsPtr.header()] == 0xa3, "invalid extensions"); extensionsPtr = certificate.firstChildOf(extensionsPtr); + _requireAsn1Tag(certificate, extensionsPtr, 0x30); Asn1Ptr extensionPtr = certificate.firstChildOf(extensionsPtr); uint256 end = extensionsPtr.content() + extensionsPtr.length(); bool basicConstraintsFound = false; @@ -442,6 +465,7 @@ contract CertManager is ICertManager { maxPathLen = -1; while (true) { + _requireAsn1Tag(certificate, extensionPtr, 0x30); Asn1Ptr oidPtr = certificate.firstChildOf(extensionPtr); bytes32 oid = certificate.keccak(oidPtr.content(), oidPtr.length()); @@ -544,6 +568,7 @@ contract CertManager is ICertManager { bytes memory signatureHints ) internal view { Asn1Ptr sigAlgoPtr = certificate.nextSiblingOf(ptr); + _requireAsn1Tag(certificate, sigAlgoPtr, 0x30); require(certificate.keccak(sigAlgoPtr.content(), sigAlgoPtr.length()) == CERT_ALGO_OID, "invalid cert sig algo"); Asn1Ptr sigPtr = certificate.nextSiblingOf(sigAlgoPtr); require(sigPtr.header() + sigPtr.totalLength() == certificate.length, "trailing cert fields"); diff --git a/test/CertManager.t.sol b/test/CertManager.t.sol index cc40cfd..fd55a60 100644 --- a/test/CertManager.t.sol +++ b/test/CertManager.t.sol @@ -166,6 +166,35 @@ contract CertManagerTest is Test { cm.verifyCACertWithHints(rootTwin, rootHash, hints); } + function test_VerifyCACertWithHints_RejectsOuterTagSubstitution() public { + vm.warp(1775145600); + CertManager cm = new CertManager(new P384Verifier()); + + bytes32 rootHash = keccak256(CB0); + bytes memory mutated = bytes.concat(CB1); + mutated[0] = 0x31; // constructed SET with the same children is not an X.509 Certificate SEQUENCE. + + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + cm.verifyCACertWithHints(mutated, rootHash, ""); + } + + function test_VerifyCACertWithHints_RejectsTbsAlgorithmTagSubstitution() public { + vm.warp(1775145600); + CertManager cm = new CertManager(new P384Verifier()); + + bytes32 rootHash = keccak256(CB0); + bytes memory mutated = bytes.concat(CB1); + Asn1Ptr root = mutated.root(); + Asn1Ptr tbsPtr = mutated.firstChildOf(root); + Asn1Ptr versionPtr = mutated.firstChildOf(tbsPtr); + Asn1Ptr serialPtr = mutated.nextSiblingOf(versionPtr); + Asn1Ptr sigAlgoPtr = mutated.nextSiblingOf(serialPtr); + mutated[sigAlgoPtr.header()] = 0x31; // constructed, but not AlgorithmIdentifier SEQUENCE. + + vm.expectRevert(CertManager.InvalidAsn1Tag.selector); + cm.verifyCACertWithHints(mutated, rootHash, ""); + } + function _verifyCA(CertManager cm, P384HintCollector collector, bytes memory cert, bytes32 parentHash) internal returns (bytes32) From b8941c3c2683284be53154c23c1d084f35e4a240 Mon Sep 17 00:00:00 2001 From: Leopold Joy Date: Fri, 26 Jun 2026 23:15:01 +0100 Subject: [PATCH 2/2] refactor: trim duplicate ASN.1 tag checks Co-authored-by: OpenCode --- src/CertManager.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/CertManager.sol b/src/CertManager.sol index b80614c..415125d 100644 --- a/src/CertManager.sol +++ b/src/CertManager.sol @@ -319,10 +319,8 @@ contract CertManager is ICertManager { bytes memory signatureHints ) internal view returns (VerifiedCert memory cert, bytes32 identity) { Asn1Ptr root = certificate.root(); - _requireAsn1Tag(certificate, root, 0x30); require(root.totalLength() == certificate.length, "invalid cert length"); Asn1Ptr tbsCertPtr = certificate.firstChildOf(root); - _requireAsn1Tag(certificate, tbsCertPtr, 0x30); (uint64 notAfter, int64 maxPathLen, bytes32 issuerHash, bytes32 subjectHash, bytes memory pubKey) = _parseTbs(certificate, tbsCertPtr, ca); @@ -414,7 +412,6 @@ contract CertManager is ICertManager { pure returns (bytes memory subjectPubKey) { - _requireAsn1Tag(certificate, subjectPublicKeyInfoPtr, 0x30); Asn1Ptr pubKeyAlgoPtr = certificate.firstChildOf(subjectPublicKeyInfoPtr); _requireAsn1Tag(certificate, pubKeyAlgoPtr, 0x30); Asn1Ptr pubKeyAlgoIdPtr = certificate.firstChildOf(pubKeyAlgoPtr);