fix: normalize ASN.1 BIT STRING key usage bits (#39)

Author: leanthebean

src/Asn1Decode.sol

@@ -118,12 +118,33 @@ library Asn1Decode {
      * @dev Extract value of bitstring node from DER-encoded structure
      * @param der The DER-encoded ASN1 structure
      * @param ptr Points to the indices of the current node
-     * @return A bitstring encoded in a uint256
+     * @return A bitstring encoded in a uint256 with the first payload octet in the least
+     *         significant byte, so X.509 bit masks are stable across multi-octet encodings.
      */
     function bitstringUintAt(bytes memory der, Asn1Ptr ptr) internal pure returns (uint256) {
         require(der[ptr.header()] == 0x03, "Not type BIT STRING");
+        require(ptr.length() > 0, "invalid BIT STRING length");
+
+        uint256 unusedBits = uint8(der[ptr.content()]);
+        require(unusedBits <= 7, "invalid BIT STRING padding");
+
         uint256 len = ptr.length() - 1;
-        return uint256(readBytesN(der, ptr.content() + 1, len) >> ((32 - len) * 8));
+        require(len <= 32, "BIT STRING too long");
+        if (len == 0) {
+            require(unusedBits == 0, "invalid BIT STRING padding");
+            return 0;
+        }
+
+        if (unusedBits != 0) {
+            uint8 unusedMask = uint8((uint256(1) << unusedBits) - 1);
+            require(uint8(der[ptr.content() + len]) & unusedMask == 0, "Non-zero unused BIT STRING bits");
+        }
+
+        uint256 value;
+        for (uint256 i = 0; i < len; ++i) {
+            value |= uint256(uint8(der[ptr.content() + 1 + i])) << (i * 8);
+        }
+        return value;
     }
 
     /*

src/CertManager.sol

@@ -469,7 +469,8 @@ contract CertManager is ICertManager {
 
     function _verifyKeyUsageExtension(bytes memory certificate, Asn1Ptr valuePtr, bool ca) internal pure {
         uint256 value = certificate.bitstringUintAt(valuePtr);
-        // bits are reversed (DigitalSignature 0x01 => 0x80, CertSign 0x32 => 0x04)
+        // X.509 KeyUsage bits are MSB-first. bitstringUintAt keeps the first KeyUsage octet in the
+        // low byte, so these masks continue to target the same bits for one- or multi-octet values.
         if (ca) {
             require(value & 0x04 == 0x04, "CertSign must be present");
         } else {

test/Asn1Decode.t.sol

@@ -28,6 +28,10 @@ contract Asn1Harness {
         return der.bitstring(der.root()).content();
     }
 
+    function bitstringUintAtRoot(bytes memory der) external pure returns (uint256) {
+        return der.bitstringUintAt(der.root());
+    }
+
     function firstChildHeader(bytes memory der) external pure returns (uint256) {
         return der.firstChildOf(der.root()).header();
     }
@@ -131,6 +135,41 @@ contract Asn1DecodeTest is Test {
         h.bitstringContent(hex"03020100"); // pad byte is 0x01, not 0x00
     }
 
+    function test_bitstringUintAt_oneByteKeyUsage() public view {
+        // X.509 KeyUsage bit 0 (digitalSignature): one content byte, 7 unused low bits.
+        assertEq(h.bitstringUintAtRoot(hex"03020780"), 0x80);
+    }
+
+    function test_bitstringUintAt_twoByteKeyUsageNormalizesFirstOctet() public view {
+        // X.509 KeyUsage bits 5 and 8 (keyCertSign | decipherOnly). The first content octet must
+        // remain in the low byte so CertManager's 0x04 keyCertSign mask still targets bit 5.
+        uint256 keyCertSignAndDecipherOnly = h.bitstringUintAtRoot(hex"0303070480");
+        assertEq(keyCertSignAndDecipherOnly & 0x04, 0x04, "keyCertSign must stay in low byte");
+        assertEq(keyCertSignAndDecipherOnly & 0x80, 0, "decipherOnly must not alias digitalSignature");
+        assertEq(keyCertSignAndDecipherOnly, 0x8004);
+    }
+
+    function test_bitstringUintAt_twoByteDecipherOnlyDoesNotAliasDigitalSignature() public view {
+        uint256 decipherOnly = h.bitstringUintAtRoot(hex"0303070080");
+        assertEq(decipherOnly & 0x80, 0, "decipherOnly must not satisfy digitalSignature");
+        assertEq(decipherOnly, 0x8000);
+    }
+
+    function test_bitstringUintAt_nonZeroUnusedBits_reverts() public {
+        vm.expectRevert("Non-zero unused BIT STRING bits");
+        h.bitstringUintAtRoot(hex"03030700ff");
+    }
+
+    function test_bitstringUintAt_invalidUnusedBits_reverts() public {
+        vm.expectRevert("invalid BIT STRING padding");
+        h.bitstringUintAtRoot(hex"03020880");
+    }
+
+    function test_bitstringUintAt_missingUnusedBits_reverts() public {
+        vm.expectRevert("invalid BIT STRING length");
+        h.bitstringUintAtRoot(hex"0300");
+    }
+
     // --- firstChildOf ---
 
     function test_firstChildOf_notConstructed_reverts() public {