Skip to content

[AUD-017] [LOW] Six of eight bitlength shift steps in conformStringToMask lack deterministic discriminating tests #87

Closed

Description

@thedavidmeister

Whole-repo audit 2026-08-24 at commit 3e76bab (audit skill 0.35.0). Pass 2 status: ledgered (audit/mutation-test-scans.json, scan of 2026-08-24T08:55:39Z at 35496d8).

src/lib/mut/LibConformString.sol:49-66

LEDGER: audit/mutation-test-scans.json: timestamp 2026-08-24T08:55:39Z, commit 35496d8, scope "whole repo" — 16 fix-merges of drift since, and the branchless bitlength derivation itself post-dates the scan (commit 9445383), with only the init and 128-step individually mutated per the brief. Judgement of the other steps at CURRENT source: the shift steps 64, 16, 8, 4, and 2 have NO deterministic discriminating test for two mutant classes. (a) Off-by-one threshold raise (shr(W,m) -> shr(W+1,m)) changes behavior only for masks whose reduced m has its top bit exactly at index W; e.g. shr(64)->shr(65) makes conformStringToMask(mask = 1<<64) derive max=63 so bit 64 is unreachable and the reroll loop never terminates. No test uses a mask reducing to top bit exactly 64, 16, 8, 4, or 2, and the fuzz (uniform uint256 mask) hits such masks with probability ~2^-192. This class is a real hang defect, not spec-equivalent. (b) Max-inflating mutants (width raise shl(w)->shl(w+1) at steps 64/16/8/4/2; threshold lower at every step including 128 and 32) keep max >= the true bitlength, so conformance, termination and determinism all still hold and only the exact keccak walk changes — but the doc contract (candidates in [0, bit length of the mask)) is violated. The only exact-walk pin in the suite is testConformStringKnownWalk at bitlength 33 (whitespace, discriminating step 32's width/threshold-raise mutants and the final step-1 add), and testConformStringHighBitOnlyMask (1<<255) pins only a LOWER bound of 256 — mod 256 and mod 257 are identical on byte candidates, so it cannot see inflation. The other pins named in the brief discriminate almost nothing: mask=1 forces the result to 0x00 for any bound >= 0, and the sparse-196 test only asserts membership in {0x41, 0xC3}, which passes for any bound >= 0x42, so it does not pin bitlength 196 at all. Fully discriminated at HEAD: step 32 (whitespace top bit is exactly 32) except its threshold-lower window, and the final step-1 add (whitespace m=1 catches spurious +1 via walk change; high-bit m=2 catches deflation via hang).

Proposed fix

Add test/lib/mut/LibConformStringSlow.sol:

// SPDX-License-Identifier: LicenseRef-DCL-1.0
// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
pragma solidity ^0.8.25;

/// @title LibConformStringSlow
/// @notice Naive reference for the conform walk written from the documented
/// spec: rerolled candidates are the top byte of keccak256(candidate, seed)
/// mod the mask's bit length, with the seed evolving across rerolls and
/// characters.
library LibConformStringSlow {
function bitLengthSlow(uint256 mask) internal pure returns (uint256 length) {
while (mask != 0) {
length++;
mask >>= 1;
}
}

function conformSlow(string memory str, uint256 mask) internal pure {
    uint256 max = bitLengthSlow(mask);
    uint256 seed = 0;
    for (uint256 i = 0; i < bytes(str).length; i++) {
        uint256 char = uint256(uint8(bytes(str)[i]));
        // forge-lint: disable-next-line(incorrect-shift)
        while ((1 << char) & mask == 0) {
            seed = uint256(keccak256(abi.encode(char, seed)));
            char = (seed >> 248) % max;
        }
        // forge-lint: disable-next-line(unsafe-typecast)
        bytes(str)[i] = bytes1(uint8(char));
    }
}

}

Add to test/src/lib/mut/LibConformString.t.sol:

/// Walks every bit length 2-256 through the branchless derivation. A
/// single-bit mask can only terminate when the derived bound reaches the
/// bit, killing any step mutant that deflates the bound (including the
/// off-by-one threshold raises, which hang exactly at their step's top
/// bit). A low bit beside the high bit makes the modulus observable, so
/// the exact walk against the spec reference kills any mutant that
/// inflates the bound.
function testConformStringBitLengthLadder() external pure {
    for (uint256 n = 1; n < 256; n++) {
        string memory s = new string(2);
        // forge-lint: disable-next-line(incorrect-shift)
        LibConformString.conformStringToMask(s, 1 << n);
        // forge-lint: disable-next-line(unsafe-typecast)
        assertEq(uint8(bytes(s)[0]), uint8(n));
        // forge-lint: disable-next-line(unsafe-typecast)
        assertEq(uint8(bytes(s)[1]), uint8(n));

        // forge-lint: disable-next-line(incorrect-shift)
        uint256 mask = (1 << n) | 1;
        string memory a = "aa";
        string memory b = "aa";
        LibConformString.conformStringToMask(a, mask);
        LibConformStringSlow.conformSlow(b, mask);
        assertEq(a, b);
    }
}

This covers every one of the 8 steps deterministically: n sweeps every top-bit window, so each step is exercised both firing and not firing, deflation fails by out-of-gas on the single-bit reachability leg, and inflation fails on the exact-walk leg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

auditAudit findinglowAudit findingpass2Audit finding

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions