A read answering more than one word is never exercised, so the word-length guard is only half covered

[thedavidmeister]

Audit finding cov-09 — dimension 2, severity LOW. Whole-repo audit pass 1 at 440e90b5.

src/lib/LibRainDeploy.sol:291-293

Problem

checkResolvedAddresses refuses a read whose returnData.length != 0x20. Two of the three cases are covered — testCheckResolvedAddressesUnreadableTargetReverts gives length == 0 and testCheckResolvedAddressesDirtyWordReverts gives exactly one dirty word — but nothing gives a read that answers with MORE than one word, which is what a consumer-supplied read of a function returning (address,address), a string, or any dynamic type produces.

The gap is mutation-visible: weakening the guard to returnData.length < 0x20 survives the entire existing suite, and the multi-word answer would then be abi.decoded down to its first word and compared as if the read had answered one address — the exact case the ResolvedAddressReadFailed docstring says it exists for ("answers with something that is not a single address-sized word").

Proposed fix

Add to test/src/lib/LibRainDeploy.t.sol. No new fixture is needed — the read is mocked on a target that really has code, so the answer's LENGTH is the only thing wrong with it:

    /// A read that answers with MORE than one word has not answered with an
    /// address either, and MUST be reported as `ResolvedAddressReadFailed`.
    /// This is a consumer that pointed a read at a function returning two
    /// values, or a dynamic type — and the answer decodes perfectly well as a
    /// word, so nothing but the length says it is not an address.
    ///
    /// The first word is the address the caller expects, so a guard that
    /// accepted any answer of at least one word would PASS this against a read
    /// that never gave a single address.
    function testCheckResolvedAddressesMultiWordAnswerReverts(bytes32 name, address account, address extra)
        external
    {
        vm.assume(account != address(0));
        (, MockResolvedOwner consumer) = deployRegistryAndConsumer(name, account);

        bytes memory twoWords = abi.encode(account, extra);
        assertEq(twoWords.length, 0x40);
        vm.mockCall(address(consumer), abi.encodeWithSignature("iOwner()"), twoWords);

        vm.expectRevert(
            abi.encodeWithSelector(
                LibRainDeploy.ResolvedAddressReadFailed.selector,
                "test_network",
                address(consumer),
                uint256(0),
                twoWords
            )
        );
        this.externalCheckResolvedAddresses("test_network", address(consumer), ownerReadCalls(), expected(account));
    }

    /// An answer SHORTER than a word but not empty is refused too — a read that
    /// returned twenty raw bytes, as the Zoltu factory itself does, is not an
    /// ABI-encoded address.
    function testCheckResolvedAddressesShortAnswerReverts(bytes32 name, address account) external {
        vm.assume(account != address(0));
        (, MockResolvedOwner consumer) = deployRegistryAndConsumer(name, account);

        bytes memory raw = abi.encodePacked(bytes20(account));
        assertEq(raw.length, 20);
        vm.mockCall(address(consumer), abi.encodeWithSignature("iOwner()"), raw);

        vm.expectRevert(
            abi.encodeWithSelector(
                LibRainDeploy.ResolvedAddressReadFailed.selector,
                "test_network",
                address(consumer),
                uint256(0),
                raw
            )
        );
        this.externalCheckResolvedAddresses("test_network", address(consumer), ownerReadCalls(), expected(account));
    }

---

Verification — this finding survived an adversarial refutation pass

Confirmed against source and by executing the mutation.

Source at src/lib/LibRainDeploy.sol:291 is exactly if (!success || returnData.length != 0x20), followed by abi.decode(returnData, (uint256)) and a uint160 range check — so the length guard is the only thing standing between a multi-word answer and it being truncated to its first word and compared as an address.

Grepped every checkResolvedAddresses test in test/src/lib/LibRainDeploy.t.sol. All reads are iOwner() (one address word) or produce an empty return (no-code target, reverting selector). Length regimes actually exercised: 0 and exactly 0x20. No test produces returnData.length > 0x20, and none produces 0 < length < 0x20 either.

Mutation proof, not inference: I edited the guard to returnData.length < 0x20 and ran forge test --match-path test/src/lib/LibRainDeploy.t.sol --match-test CheckResolvedAddresses. Result: 10 passed, 2 failed — and both failures are vm.createSelectFork: environment variable ARBITRUM_RPC_URL / BASE_RPC_URL not found, i.e. missing local .env RPC endpoints on the two fork-based OnNetworks tests, not mutation kills. Every non-fork test passed under the weakened guard, so the mutant survives. Source restored to != 0x20 afterwards and verified.

None of the refutation grounds apply: the behaviour exists as described, the cited lines say what the finding claims, no sibling test or documented convention covers the multi-word case, and this is not a restatement of an org convention. The docstring at lines 288-290 explicitly states the error's subject is a read that "answers with something that is not a single address-sized word", so the uncovered case is one the code intends to handle.

Severity LOW is correct: the guard as written is currently CORRECT, so there is no live production defect — the value at risk is only the regression risk of an unpinned guard in a deploy-verification helper that runs in scripts, not on-chain. Keeping LOW.