Skip to content

predictDeterministicAddressOpenSalt NatSpec claims non-zero code implies initialized state, but the clone has code before initialize runs #62

Description

@thedavidmeister

Unit

LibICloneableFactoryV4.predictDeterministicAddressOpenSalt (src/lib/LibICloneableFactoryV4.sol:223-229) and the NewClone-then-initialize ordering in cloneAndInitialize (src/lib/LibICloneableFactoryV4.sol:157-168).

Intent oracle

src/interface/ICloneableFactoryV4.sol:218-222, the closing claim of predictDeterministicAddressOpenSalt:

A non-zero code size at the returned address means this exact (implementation, data, salt) has already been deployed by somebody and cloneDeterministicOpenSalt will revert there. Since nothing else can be deployed there, what occupies it is the clone that was asked for, initialized with the bytes that were asked for.

This is the load-bearing sentence for a consumer that pins an open-salt address: it says a code-size check at the predicted address is sufficient to conclude the clone is present and carries the intended state.

Violated property

The clause "initialized with the bytes that were asked for" does not hold for the whole window in which the address has code. cloneAndInitialize deploys via CREATE2 (line 158), emits NewClone (line 163), and only then calls initialize (line 166). Between the CREATE2 and the initialize return, the clone is at its final predicted address with its full 45-byte EIP-1167 runtime and zero initialized state.

Any third party that initialize reaches — directly or transitively — observes the pinned address with non-zero code and uninitialized state, and the documented inference ("what occupies it is … initialized with the bytes that were asked for") is false for that observer.

This matters because the whole open-salt threat model assumes the deployer is untrusted while the address commits to (implementation, data, salt). The deployer picks implementation, and any implementation whose initialize calls out opens the window at an address a consumer may already be watching.

Verified repro

Against TestCloneFactory at c1c2afd:

contract Peeker {
    uint256 public sCodeLen;
    bytes public sDataDuringInit;
    function peek(address clone) external {
        sCodeLen = clone.code.length;
        (bool ok, bytes memory ret) = clone.staticcall(abi.encodeWithSignature("sData()"));
        if (ok) { sDataDuringInit = abi.decode(ret, (bytes)); }
    }
}

contract Callout is ICloneableV2 {
    bytes public sData;
    Peeker public immutable I_PEEKER;
    constructor(Peeker peeker) { I_PEEKER = peeker; }
    function initialize(bytes memory data) external returns (bytes32) {
        I_PEEKER.peek(address(this));   // third party observes the clone here
        sData = data;
        return ICLONEABLE_V2_SUCCESS;
    }
}
bytes memory data = hex"deadbeef";
address predicted = I_FACTORY.predictDeterministicAddressOpenSalt(address(impl), data, bytes32(uint256(7)));
address child     = I_FACTORY.cloneDeterministicOpenSalt(address(impl), data, bytes32(uint256(7)));
assertEq(child, predicted);

Observed:

observed:             0xf2De72FFED8389F8E8beCac2dd34dAa2B226507b   (== predicted == child)
code len during init: 45
read reverted:        no
sData during init:    0x          <- NOT the bytes that were asked for
sData after init:     0xdeadbeef

So at the predicted address, mid-transaction: full EIP-1167 code present, sData() callable and empty.

Same window exists on the cloneDeterministic path — the ordering is in the shared tail — but there the NatSpec makes no equivalent claim, so the mismatch is specific to the open-salt predict… docs.

Triage framing

Flagging, not adjudicating. The readings I can see:

  • By design, docs are about end-of-transaction state. Every external observer at a block boundary sees either no code (the deploy reverted, atomicity held) or a fully initialized clone; the interleaved observer is inside a transaction the implementation itself authored. Under this reading the sentence wants a qualifier ("once the deploying transaction has completed"), not a code change.
  • A defect in the ordering. NewClone is emitted before initialize for no stated reason; emitting after a successful initialize would not close the observation window (the CREATE2 still precedes initialize) but would stop the event from asserting a clone that is not yet initialized. Genuinely closing the window would need the initialization to happen in the clone's constructor, which EIP-1167 cannot do.

I do not know which the authors intend, and the interface text is currently unqualified, so I am recording it rather than deciding.

Also noted while here

predictDeterministicAddressOpenSalt and predictDeterministicAddress apply no checkImplementationCode guard, so they return an address for a zero-code implementation that cloneDeterministic* can never deploy to. That looks intentional for a pure prediction function and is mentioned only for completeness.

Found by adversarial review during AMT group g3-libicloneablefactoryv4-clone.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    adversarialFound by adversarial reviewauditAudit finding

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions