Skip to content

deployZoltu reports the factory's revert data as deployedAddress on the failure path #17

Description

@thedavidmeister

Found by an adversarial mutation-testing pass over the whole repo at 4422e29
(2 commits past sol-v0.1.4).

Summary

LibRainDeploy.deployZoltu reads its call output buffer unconditionally:

success := call(gas(), zoltuFactory, 0, add(creationCode, 0x20), mload(creationCode), 12, 20)
deployedAddress := mload(0)

The EVM copies return data into the output region for failed calls as well
as successful ones. So when the factory call reverts with at least 20 bytes of
revert data, the first 20 bytes of that revert data land at memory [12, 32)
and become deployedAddress — a value the factory never returned as an
address.

That value is then reported to the operator in four places on the failure path:

console2.log("Zoltu deployment failed. Success:", success, "Deployed Address:", deployedAddress);
console2.log("Code length at Deployed Address:", deployedAddress.code.length);
console2.log("Codehash at Deployed Address:");
console2.logBytes32(deployedAddress.codehash);
revert DeployFailed(success, deployedAddress);

DeployFailed's NatSpec describes deployedAddress as the deployed address,
so an operator debugging a failed deploy is shown a fabricated address together
with a real code length and a real code hash for that fabricated address.

Impact

Diagnostics only — the guard short-circuits on !success, so a failed call can
never be mistaken for a successful deployment. Rate accordingly.

Reachability against the canonical factory is limited: 0x7A0D…D12 is a
keyless (Nick's method) deployment, so only that exact bytecode can ever exist
there, and it only ever reverts with empty data. The path is reachable via
etchZoltuFactory and etched test doubles, and by any caller using
deployZoltu directly — it is internal and has no factory-codehash guard of
its own; only deployToNetworks checks ZOLTU_FACTORY_CODEHASH.

Repro

Verified against 4422e29.

/// Factory stand-in whose calls always fail, reverting with exactly the twenty
/// bytes of its own address.
contract MockAddressRevertingFactory {
    fallback() external {
        bytes20 self = bytes20(address(this));
        assembly ("memory-safe") {
            mstore(0, self)
            revert(0, 20)
        }
    }
}

function testRevertDataLeaksIntoReportedAddress() external {
    vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE);
    MockAddressRevertingFactory mock = new MockAddressRevertingFactory();
    vm.etch(LibRainDeploy.ZOLTU_FACTORY, address(mock).code);

    // The reported address is the factory itself, taken from its revert data,
    // not any address the factory returned.
    vm.expectRevert(
        abi.encodeWithSelector(LibRainDeploy.DeployFailed.selector, false, LibRainDeploy.ZOLTU_FACTORY)
    );
    this.externalDeployZoltu(type(MockDeployable).creationCode);
}

Passes, with:

Zoltu deployment failed. Success: false Deployed Address: 0x7A0D94F55792C434d74a40883C6ed8545E406D12
Code length at Deployed Address: 30
Codehash at Deployed Address:
0xbdc7c1be5540fa64d6e4303631718a86557e9474ebcd797ca21f21150f90c2af

Proposed fix

Only read the output buffer when the call succeeded, so a failed call reports
the zero address rather than revert bytes:

assembly ("memory-safe") {
    mstore(0, 0)
    success := call(gas(), zoltuFactory, 0, add(creationCode, 0x20), mload(creationCode), 12, 20)
    if success { deployedAddress := mload(0) }
}

The accompanying coverage PR adds
testDeployZoltuRevertsWhenFactoryCallFailsWithAddressData, which pins that a
failed factory call must never yield a deployment. It deliberately asserts only
the DeployFailed selector, not the reported address, so it does not enshrine
the behaviour described here and stays green under the fix above.

Activity

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

Metadata

Metadata

Labels

auditAudit finding

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions