LibHexString.bytesToHex documents a hex-charset guarantee it does not check, and the fuzz oracle encodes the same gap

[thedavidmeister]

Audit scope: whole-repo, commit 7aa85a4

Dimension 3 (documentation), also 5 (correctness/intent) · low

src/lib/LibHexString.sol:7-12, 22-24;
test/lib/LibHexString.bytesToHex.t.sol:368-393 (test/src/lib/ after #56)

Problem

Both the error's NatSpec and the function's say the Vm is required to return
"0x followed by exactly two hexadecimal characters per input byte", and
that a return which is not that reverts. Only the length and the two prefix
bytes are checked — 0xZZZZ for two bytes of data is accepted and reaches
generated source as hex"ZZZZ", which fails to compile with no mention of the
Vm that produced it.

The fuzz property testBytesToHexRejectsEveryNonConformingVmOutput states it
derives conformance "from the definition of toString(bytes) rather than read
back off the library", but its conforms predicate (line 381) encodes only
length + prefix, so the test enshrines the gap and would itself have to change
if the library were tightened to match its docs.

Proposed fix

Check the payload's charset alongside the prefix, and tighten the test oracle to
the definition the docs state. In bytesToHex, after the prefix check and
before stripped := 1:

                    // Every remaining character must be a lower case hex
                    // nibble, which is what `toString(bytes)` is defined to
                    // return. Without this a non conforming `Vm` reaches
                    // generated source as a `hex"..."` literal that does not
                    // compile, and nothing names the `Vm` as the cause.
                    let valid := 1
                    for { let i := 2 } lt(i, len) { i := add(i, 1) } {
                        let c := byte(0, mload(add(add(hexString, 0x20), i)))
                        // `0`-`9` or `a`-`f`.
                        if iszero(or(and(gt(c, 0x2f), lt(c, 0x3a)), and(gt(c, 0x60), lt(c, 0x67)))) {
                            valid := 0
                            break
                        }
                    }

and gate the strip on valid. Then extend the test's conforms with the same
per-character rule spelled out in Solidity.

If the check is declined instead, the two docstrings must be narrowed to "0x
followed by exactly two characters per input byte" and the false revert claim
dropped.