bitstringAt() reverts on bitstrings that have length not multiple of 8

[JasonVranek]

The bitstringAt() function is used to parse a DER-encoded bitstring from raw DER data. In DER, the contents octet consists of an “initial octet” and the “subsequent octets”, where subsequent octets contain the bitstring data and the initial octet indicates the number of unused bits in the last subsequent octet (see [section 8.6 of ITU-T Rec. X.690](https://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf)). However, bitstringAt() requires that the initial octet is equal to 0, so that there are no unused bits in the last subsequent octet. This constrains the length of the bitstring to be a multiple of 8. If there are unused bits, then the function will revert.

Definition of bitstringAt()

function bitstringAt(bytes memory der, uint256 ptr) internal pure returns (bytes memory) {
    require(der[ptr.ixs()] == 0x03, "Not type BIT STRING");
    // Only 00 padded bitstr can be converted to bytestr!
    require(der[ptr.ixf()] == 0x00);
    uint256 valueLength = ptr.ixl() + 1 - ptr.ixf();
    return der.substring(ptr.ixf() + 1, valueLength - 1);
}

Impact

Currently, the only use of bitstringAt() is to extract the DER-encoded RSA public key of the leaf certificate. This currently has no impact as DER data is always made up of octets (not individual bits). However, if in the future bitstringAt() is used to read bitstrings that are not always guaranteed to have lengths that are multiples of 8, then the function could revert and cause the RAVe smart contracts to reject valid leaf certificates.

Recommendation

The developers may want to change bitstringAt() to correctly handle the last subsequent octet if they intend for RAVe to read other bitstring fields in the future.

[robertsdotpm]

Good catch, we can fix this in the future if the need arises.