From 9d29c9967fc3285122d4781ce68c71de67df7fa4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 15 Aug 2026 12:51:19 +0000 Subject: [PATCH] test: cover every answer length the resolved-address read guard refuses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `checkResolvedAddresses` refuses a read whose answer is not exactly one word, and only two of the lengths that can be were exercised: zero, from a target with no code, and exactly one word, from a real address getter. Nothing produced an answer LONGER than a word — what a read pointed at a getter returning `(address, address)`, or any dynamic type, answers with — and nothing produced one shorter but non-empty either. Both gaps were mutation-visible in opposite directions: weakening the guard to `returnData.length < 0x20` accepted the long answer, decoded it down to its first word and compared that as if the read had answered one address, and `> 0x20` accepted the short one into a decoder that reverts with no data of its own — the bare, undiagnosable revert `ResolvedAddressReadFailed` exists to replace. Covered as the rule rather than as two examples: one test fuzzes the whole tail past the first word, so 0x21 bytes is as much an instance as 0x40, and the other fuzzes every length between an empty answer and a whole word. The long answer's leading word is exactly the address the caller expects, so nothing but the length is wrong with it and a guard that accepted any answer of at least one word PASSES against a read that never gave a single address. `MockRawAnswerOwner` is what makes those answers reachable from a contract rather than from a cheatcode. It returns in assembly because the length is the point: every Solidity return type ABI-encodes to a whole number of words, so nothing declared can answer with twenty bytes — which is exactly what the Zoltu factory answers with. Closes https://github.com/rainlanguage/rain.deploy/issues/62 Co-Authored-By: Claude Opus 5 (1M context) --- test/concrete/MockRawAnswerOwner.sol | 34 +++++++++++++++++ test/src/lib/LibRainDeploy.t.sol | 57 ++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 test/concrete/MockRawAnswerOwner.sol diff --git a/test/concrete/MockRawAnswerOwner.sol b/test/concrete/MockRawAnswerOwner.sol new file mode 100644 index 0000000..320b799 --- /dev/null +++ b/test/concrete/MockRawAnswerOwner.sol @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +/// @title MockRawAnswerOwner +/// @notice Answers the read `MockResolvedOwner` answers, with an arbitrary byte +/// string rather than with one ABI-encoded address. A call answers with as many +/// bytes as the callee likes, and contracts whose answer is not exactly one word +/// are ordinary: a getter declared `returns (address, address)`, or returning a +/// dynamic type, answers with more than a word, and one that returns in assembly +/// answers with whatever it holds — the Zoltu factory itself answers with the +/// twenty raw bytes of an address and nothing else. +/// +/// Assembly, and no declared return type, because the LENGTH is the whole point: +/// every Solidity return type ABI-encodes to a whole number of words, so nothing +/// declared can answer with twenty bytes. +contract MockRawAnswerOwner { + /// The bytes this contract answers every read with, verbatim. + bytes internal sAnswer; + + /// @param answer The bytes to answer with. + constructor(bytes memory answer) { + sAnswer = answer; + } + + /// The selector `MockResolvedOwner` answers, returning `sAnswer` as the + /// whole of the return data. + function iOwner() external view { + bytes memory answer = sAnswer; + assembly ("memory-safe") { + return(add(answer, 0x20), mload(answer)) + } + } +} diff --git a/test/src/lib/LibRainDeploy.t.sol b/test/src/lib/LibRainDeploy.t.sol index 23d302c..6f08da8 100644 --- a/test/src/lib/LibRainDeploy.t.sol +++ b/test/src/lib/LibRainDeploy.t.sol @@ -9,6 +9,7 @@ import {AddressRegistry, ADDRESS_REGISTRY_ROOT} from "../../../src/concrete/Addr import {MockAddressRevertingFactory} from "../../concrete/MockAddressRevertingFactory.sol"; import {MockResolvedOwner} from "../../concrete/MockResolvedOwner.sol"; import {MockDirtyWordOwner} from "../../concrete/MockDirtyWordOwner.sol"; +import {MockRawAnswerOwner} from "../../concrete/MockRawAnswerOwner.sol"; import {MockAddressRevertingFactory} from "../../concrete/MockAddressRevertingFactory.sol"; import {MockDeployable} from "../../concrete/MockDeployable.sol"; import {MockDeployableV2} from "../../concrete/MockDeployableV2.sol"; @@ -954,6 +955,62 @@ contract LibRainDeployTest is Test { ); } + /// A read that answers with MORE than one word has not answered with an + /// address either, whatever its leading word says, and MUST be reported as + /// `ResolvedAddressReadFailed`. This is the consumer that pointed a read at + /// a getter returning two values, or a dynamic type: the answer's first word + /// decodes perfectly well, so nothing but the length says it is not an + /// address. + /// + /// That first word is exactly the address the caller expects, so a guard + /// that accepted any answer of at least one word would decode it and PASS + /// this against a read that never answered with a single address. Fuzzed + /// over the whole tail rather than over one extra word, because the rule is + /// about every length that is not one word, not about a `(address,address)` + /// return in particular — an answer of 0x21 bytes is no more an address than + /// one of 0x40. + function testCheckResolvedAddressesLongAnswerReverts(address account, bytes memory tail) external { + vm.assume(tail.length > 0); + bytes memory answer = bytes.concat(abi.encode(account), tail); + MockRawAnswerOwner target = new MockRawAnswerOwner(answer); + + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.ResolvedAddressReadFailed.selector, "test_network", address(target), uint256(0), answer + ) + ); + this.externalCheckResolvedAddresses("test_network", address(target), ownerReadCalls(), expected(account)); + } + + /// An answer SHORTER than a word is not an ABI-encoded address either, and + /// MUST be reported as `ResolvedAddressReadFailed` rather than reverting + /// inside the decoder with no data of its own. A read pointed at a getter + /// that answers in raw bytes lands here — the Zoltu factory itself answers + /// with the twenty bytes of an address and nothing else. + /// + /// Fuzzed across every length between an answer of nothing and an answer of + /// one word. Zero is the case a target with no code produces, and 0x20 is an + /// address, so both ends are excluded and everything between them is the + /// regime nothing else exercises. + function testCheckResolvedAddressesShortAnswerReverts(address account, bytes32 tail, uint256 lengthSeed) external { + uint256 length = bound(lengthSeed, 1, 0x1f); + // The raw bytes of the address first, so length 20 is exactly the shape + // the Zoltu factory answers with, then arbitrary bytes to fill. + bytes memory source = abi.encodePacked(account, tail); + bytes memory answer = new bytes(length); + for (uint256 i = 0; i < length; i++) { + answer[i] = source[i]; + } + MockRawAnswerOwner target = new MockRawAnswerOwner(answer); + + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.ResolvedAddressReadFailed.selector, "test_network", address(target), uint256(0), answer + ) + ); + this.externalCheckResolvedAddresses("test_network", address(target), ownerReadCalls(), expected(account)); + } + /// `checkResolvedAddresses` MUST revert when the reads and expected /// addresses do not pair up, rather than checking the shorter of the two. function testCheckResolvedAddressesLengthMismatchReverts(uint8 readCallsLength, uint8 expectedLength) external {