Skip to content

testRequireContractNameAcceptedNamesCannotTraverse asserts nothing on 95% of its runs #58

Description

@thedavidmeister

Audit scope: whole-repo, commit 7aa85a4

Dimension 2 (test coverage) / 5 (correctness/intent) · medium

Location: test/src/lib/LibCodeGen.requireContractName.t.sol:159-170

Path note: PR #56 is open and unmerged as of filing. On main today this file is test/lib/LibCodeGen.requireContractName.t.sol; it becomes test/src/lib/LibCodeGen.requireContractName.t.sol after #56 merges.

Problem

try … catch {} — the empty catch means a rejected name passes the test having
asserted nothing, and the catch is untyped, so an unexpected revert also counts
as a pass. Measured at --fuzz-seed 1, 2048 runs: 101 accepted, 1947 silently
skipped. The property named in the title (accepted ⟹ no filesystem-meaningful
byte) is proven on 4.9% of the budget.

Constructing names with nameFromSeedSlow would not fix it — that would make the
assertion a property of the generator's alphabet rather than of
requireContractName.

Proposed fix

Prove it deterministically and completely instead.
testRequireContractNameMatchesAlphabet (line 207) already establishes accepted
⟹ every byte ∈ SLOW_TAIL_ALPHABET over arbitrary bytes; the missing half is a
property of the alphabet itself. Delete lines 155-170 and add (verified passing):

/// The identifier alphabet contains no byte that means anything to a
/// filesystem, so no accepted name can leave the directory it is interpolated
/// into. Stated over the alphabet rather than waiting for the fuzzer to produce
/// an accepted name; with `testRequireContractNameMatchesAlphabet`, which pins
/// accepted names to that alphabet, this covers the whole accepted set.
function testRequireContractNameAlphabetCannotTraverse() external pure {
    bytes memory tail = bytes(SLOW_TAIL_ALPHABET);
    for (uint256 i = 0; i < tail.length; i++) {
        assertNotEq(uint8(tail[i]), uint8(bytes1("/")), "separator in the alphabet");
        assertNotEq(uint8(tail[i]), uint8(bytes1("\\")), "backslash in the alphabet");
        assertNotEq(uint8(tail[i]), uint8(bytes1(".")), "dot in the alphabet");
        assertNotEq(uint8(tail[i]), uint8(bytes1(hex"00")), "nul in the alphabet");
    }
}

Activity

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

Metadata

Metadata

Labels

auditAudit findingmediumAudit findingpass2Audit findingpass5Audit finding

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions