From 080dc31a3ddb4215bac7061cbec58833b0eb83ea Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 16 Aug 2026 18:19:41 +0000 Subject: [PATCH 1/2] Check the hex charset in LibHexString.bytesToHex The docs on `UnexpectedHexString` and on `bytesToHex` both state the `Vm` must return "0x" followed by two hexadecimal characters per input byte, but only the length and the prefix were checked, so `0xZZZZ` reached generated source as `hex"ZZZZ"`. Every character behind the prefix is now checked against the lower case hexadecimal charset, and the fuzz oracle derives conformance from the same rule. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibHexString.sol | 50 ++++++++++---- test/lib/LibHexString.bytesToHex.t.sol | 92 ++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 13 deletions(-) diff --git a/src/lib/LibHexString.sol b/src/lib/LibHexString.sol index a6c412c..e89afe9 100644 --- a/src/lib/LibHexString.sol +++ b/src/lib/LibHexString.sol @@ -4,9 +4,12 @@ pragma solidity ^0.8.25; import {Vm} from "forge-std-1.16.1/src/Vm.sol"; -/// Thrown when the `Vm` handed to `bytesToHex` does not return the string that -/// `toString(bytes)` is defined to return, which is "0x" followed by exactly two -/// hexadecimal characters per input byte. +/// Thrown when the `Vm` handed to `bytesToHex` does not return a string shaped +/// the way `toString(bytes)` is defined to shape its return, which is "0x" +/// followed by exactly two lower case hexadecimal characters per input byte. +/// Only the shape is checked. A return of the right shape that encodes bytes +/// other than the input cannot be told from a correct one without redoing the +/// conversion, so it is accepted. /// @param hexString The string the `Vm` returned. /// @param expectedLength The length the returned string was required to have. error UnexpectedHexString(string hexString, uint256 expectedLength); @@ -20,12 +23,15 @@ library LibHexString { /// accept the prefix, such as in `hex"..."` literals. /// /// Reverts with `UnexpectedHexString` if the `Vm` does not return "0x" - /// followed by two hexadecimal characters per input byte. That is what - /// foundry's own `Vm` always returns, but `vm` is a parameter, so the string - /// the prefix is stripped from is whatever the caller's `Vm` hands back. - /// Stripping two characters from a shorter string underflows the length - /// word, and stripping them from an unprefixed one silently discards two - /// characters of real data into generated source that still compiles. + /// followed by two lower case hexadecimal characters per input byte. That is + /// what foundry's own `Vm` always returns, but `vm` is a parameter, so the + /// string the prefix is stripped from is whatever the caller's `Vm` hands + /// back. Stripping two characters from a shorter string underflows the + /// length word, stripping them from an unprefixed one silently discards two + /// characters of real data into generated source that still compiles, and a + /// character outside the hexadecimal charset reaches generated source inside + /// a `hex"..."` literal that does not compile, naming the generated file + /// rather than the `Vm` that produced it. /// @param vm The Vm instance used for conversion. /// @param data The bytes array to convert. /// @return The hexadecimal string representation of the bytes array. @@ -33,6 +39,23 @@ library LibHexString { string memory hexString = vm.toString(data); uint256 expectedLength = data.length * 2 + 2; + // Every character behind the prefix must be a lower case hexadecimal + // nibble, which is what `toString(bytes)` is defined to emit. The scan + // starts past the prefix, so it does nothing at all on a string too + // short to hold one, which the length check below rejects. It reads + // through solidity's bounds checked indexing rather than in the + // assembly block, because this is a build time function where the gas a + // hand rolled scan would save is not a consideration. + bytes memory returned = bytes(hexString); + bool hexCharset = true; + for (uint256 i = 2; i < returned.length; i++) { + uint8 c = uint8(returned[i]); + if (!((c >= 0x30 && c <= 0x39) || (c >= 0x61 && c <= 0x66))) { + hexCharset = false; + break; + } + } + bool stripped; assembly ("memory-safe") { let len := mload(hexString) @@ -41,10 +64,11 @@ library LibHexString { // at least 2, so a length that matches it guarantees the word read // below is within the string's own allocation. if eq(len, expectedLength) { - // The first two bytes of the data word against "0x". One word - // load rather than two indexed byte reads, and it reuses the - // pointer the strip already needs. - if eq(shr(240, mload(add(hexString, 0x20))), 0x3078) { + // The first two bytes of the data word against "0x", alongside + // the charset of everything behind them. One word load rather + // than two indexed byte reads, and it reuses the pointer the + // strip already needs. + if and(hexCharset, eq(shr(240, mload(add(hexString, 0x20))), 0x3078)) { // Remove the leading 0x, which solidity does not always // accept — such as in `hex"..."` literals. let newHexString := add(hexString, 2) diff --git a/test/lib/LibHexString.bytesToHex.t.sol b/test/lib/LibHexString.bytesToHex.t.sol index a4163b9..de5545e 100644 --- a/test/lib/LibHexString.bytesToHex.t.sol +++ b/test/lib/LibHexString.bytesToHex.t.sol @@ -341,6 +341,91 @@ contract LibHexStringBytesToHexTest is Test { external_.bytesToHex(badVm, ""); } + /// The payload is checked against the hexadecimal charset, not merely + /// counted. A return of the right length behind the right prefix whose + /// payload is not hexadecimal reaches generated source inside a `hex"..."` + /// literal that does not compile, and the compiler names the generated file + /// rather than the `Vm` that produced the characters. + function testBytesToHexRevertsOnNonHexVmOutput() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0xZZZZ"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0xZZZZ", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// `toString(bytes)` is defined to return lower case, so upper case nibbles + /// are a non conforming return even though they name the same bytes. + function testBytesToHexRevertsOnUpperCaseVmOutput() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0xAABB"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0xAABB", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// The character immediately after the prefix is checked, so a scan that + /// begins one character late does not accept this. + function testBytesToHexRevertsOnNonHexFirstPayloadCharacter() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0xZabb"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0xZabb", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// The final character of the payload is checked, so a scan that stops one + /// character early does not accept this. + function testBytesToHexRevertsOnNonHexLastPayloadCharacter() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0xaabZ"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0xaabZ", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// `/` is the character immediately below `0`, the bottom of the digit + /// range, so only the lower bound of that range rejects it. + function testBytesToHexRevertsOnCharacterBelowDigitRange() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0x/abb"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0x/abb", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// `:` is the character immediately above `9`, the top of the digit range, + /// so only the upper bound of that range rejects it. + function testBytesToHexRevertsOnCharacterAboveDigitRange() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0x:abb"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0x:abb", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// A backtick is the character immediately below `a`, the bottom of the + /// letter range, so only the lower bound of that range rejects it. + function testBytesToHexRevertsOnCharacterBelowLetterRange() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0x`abb"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0x`abb", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// `g` is the character immediately above `f`, the top of the letter range, + /// so only the upper bound of that range rejects it. + function testBytesToHexRevertsOnCharacterAboveLetterRange() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm badVm = Vm(address(new NonConformingVm("0xgabb"))); + vm.expectRevert(abi.encodeWithSelector(UnexpectedHexString.selector, "0xgabb", uint256(6))); + external_.bytesToHex(badVm, hex"aabb"); + } + + /// Every character of the hexadecimal charset is accepted, which includes + /// the four that sit on the boundaries of the two accepted ranges: `0`, `9`, + /// `a` and `f`. The charset check narrows what is accepted, so what it must + /// not narrow is pinned alongside it. + function testBytesToHexAcceptsEveryHexNibble() external { + LibHexStringExternal external_ = new LibHexStringExternal(); + Vm conformingVm = Vm(address(new NonConformingVm("0x0123456789abcdef"))); + assertEq(external_.bytesToHex(conformingVm, hex"0011223344556677"), "0123456789abcdef"); + } + /// The check gates on the shape of the returned string, not on the identity /// of the `Vm`. A conforming `Vm` that is not foundry's is still stripped /// and returned, so the guard does not quietly narrow the parameter to the @@ -379,6 +464,13 @@ contract LibHexStringBytesToHexTest is Test { // The length equality implies at least 2 characters, so the prefix // reads only happen once indexing them is in bounds. bool conforms = returned.length == expectedLength && returned[0] == bytes1("0") && returned[1] == bytes1("x"); + // `toString(bytes)` is defined to emit two lower case hexadecimal + // nibbles per input byte, so a payload character outside that charset is + // as non conforming as a wrong length or a missing prefix. + for (uint256 i = 2; conforms && i < returned.length; i++) { + uint8 c = uint8(returned[i]); + conforms = (c >= 0x30 && c <= 0x39) || (c >= 0x61 && c <= 0x66); + } if (conforms) { bytes memory expected = new bytes(returned.length - 2); From eb344aacadff572b68118bbf3ecd9449272a99ee Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 17 Aug 2026 05:04:59 +0000 Subject: [PATCH 2/2] test: correct the property docstring the charset check falsifies `testBytesToHexStripsOrRevertsForEveryVmOutput` said its constructed payload is "arbitrary in everything except the length and prefix the library actually checks". This PR adds a charset check to `bytesToHex`, so the library now checks the shape rather than the length and prefix alone, and an arbitrary `filler` conforms only rarely: measured at 2048 runs against seeds 1, 2 and 3 the accepted half is reached 57, 56 and 52 times here against 1021, 991 and 1036 on `main`. The construction itself is untouched. Mapping `filler` into `0`-`9a`-`f` to restore the even split belongs to the PR that built the accept arm. Co-Authored-By: Claude Opus 5 (1M context) --- test/src/lib/LibHexString.bytesToHex.t.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/src/lib/LibHexString.bytesToHex.t.sol b/test/src/lib/LibHexString.bytesToHex.t.sol index 561d728..3fd7ec3 100644 --- a/test/src/lib/LibHexString.bytesToHex.t.sol +++ b/test/src/lib/LibHexString.bytesToHex.t.sol @@ -468,7 +468,11 @@ contract LibHexStringBytesToHexTest is Test { /// an unconstructed return conforms 0 times in 2048 runs. `conforming` /// picks which half of the property a run aims at, and the payload is /// filled from `filler` so that an accepted string is arbitrary in - /// everything except the length and prefix the library actually checks. + /// everything except the shape the library actually checks. That shape + /// includes the hexadecimal charset, which an arbitrary `filler` almost + /// never satisfies, so a constructed payload still lands in the rejected + /// half most of the time: the accepted half is reached on the order of 55 + /// runs in 2048 rather than on the order of 1024. /// Whether a string conforms is still read off its own characters below, so /// a `filler` that happens to conform is checked against the accepted half /// rather than expected to revert.