BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: low

Factory does not verify `poolImplementation` holds code

Author Revealed upon completion

Root + Impact

Description

  • The factory stores a poolImplementation address and deploys every pool as an EIP-1167 minimal-proxy clone of it. The address is written at deployment in initialize and can be changed by the owner via setPoolImplementation; both paths are expected to point at the canonical ConfidencePool implementation contract.

  • Both write paths validate the address only against address(0). Neither checks that the address actually contains contract code, so a codeless address is accepted and used for cloning. A clone of a codeless implementation forwards every call by delegatecall into empty code, which returns success with no return data — so createPool completes, registers the clone, and emits PoolCreated for a pool that can never function.

function initialize(address safeHarborRegistry_, address poolImplementation_, address defaultOutcomeModerator_)
external
initializer
{
if (safeHarborRegistry_ == address(0)) revert ZeroAddress();
@> if (poolImplementation_ == address(0)) revert ZeroAddress(); // only zero-address checked, not code
if (defaultOutcomeModerator_ == address(0)) revert ZeroAddress();
...
poolImplementation = poolImplementation_;
}
​
function setPoolImplementation(address newPoolImplementation) external onlyOwner {
@> if (newPoolImplementation == address(0)) revert ZeroAddress(); // only zero-address checked, not code
address old = poolImplementation;
poolImplementation = newPoolImplementation;
emit PoolImplementationUpdated(old, newPoolImplementation);
}
​
function createPool(...) external whenNotPaused returns (address pool) {
...
@> pool = Clones.cloneDeterministic(poolImplementation, salt); // Clones does not check for code either
IConfidencePool(pool).initialize(...); // delegatecall into empty code "succeeds"
_poolsByAgreement[agreement].push(pool); // inert pool is recorded
emit PoolCreated(...);
}

Risk

Likelihood:

  • Occurs whenever poolImplementation is set — at initialize or through setPoolImplementation — to an address that holds no code: a mistyped or wrong address, or a counterfactually-computed CREATE2 address that has not been deployed on the target chain.

  • The only guard on both write paths is the zero-address check, so any nonzero codeless address is accepted and then cloned by every subsequent createPool call with no further validation.

Impact:

  • createPool succeeds and records the clone (emitting PoolCreated) even though the pool can never function, so the misconfiguration is not surfaced at creation time and can be discovered only when the pool is exercised.

  • Pools cloned during the window are inert: calls delegatecall into empty code and return success with no data, so participants can approve and interact with a registered pool address that holds no logic and moves no funds.

Proof of Concept

Both tests pass (forge test --match-path test/unit/PoCImplementationCodeCheck.t.sol). The first shows a codeless implementation is accepted, createPool succeeds and registers the pool, and the pool is inert; the second is the contrast with a real implementation.

function test_PoC_createPoolSucceedsWithCodelessImplementationAndYieldsInertPool() external {
// An address that has never been deployed to — it contains no code.
address codeless = makeAddr("codelessImplementation");
assertEq(codeless.code.length, 0, "precondition: implementation has no code");
​
// The factory accepts it: only the zero-address check exists, no code-length check.
factory.setPoolImplementation(codeless);
assertEq(factory.poolImplementation(), codeless);
​
// createPool succeeds and registers the pool even though the implementation is codeless.
// The clone's initialize() is a delegatecall into empty code, which returns success.
uint256 expiry = block.timestamp + 31 days;
vm.prank(agreementOwner);
address created = factory.createPool(address(agreement), address(token), expiry, ONE, recovery, _scope());
​
assertEq(factory.poolCountByAgreement(address(agreement)), 1, "a non-functional pool was registered");
assertEq(factory.getPoolsByAgreement(address(agreement))[0], created);
​
// A functional pool's outcomeModerator() returns a 32-byte address (see the contrast test).
// Here the call delegatecalls into empty code, which "succeeds" with no return data.
(bool ok, bytes memory data) = created.staticcall(abi.encodeWithSignature("outcomeModerator()"));
assertTrue(ok, "call to inert pool reverted (expected silent success)");
assertEq(data.length, 0, "inert pool returned data (a functional pool would return 32 bytes)");
}
[PASS] test_PoC_createPoolSucceedsWithCodelessImplementationAndYieldsInertPool()
[PASS] test_PoC_contrast_realImplementationYieldsFunctionalPool()
Suite result: ok. 2 passed; 0 failed; 0 skipped

Recommended Mitigation

Reject implementation addresses that contain no code at both write sites, so a misconfiguration reverts where it is introduced instead of surfacing later as a broken pool. A code-length check confirms only that the address is a deployed contract, not that it is the intended ConfidencePool implementation, so deployment procedures should still verify the implementation identity independently.

+ error InvalidImplementation();
+
function initialize(address safeHarborRegistry_, address poolImplementation_, address defaultOutcomeModerator_)
external
initializer
{
if (safeHarborRegistry_ == address(0)) revert ZeroAddress();
- if (poolImplementation_ == address(0)) revert ZeroAddress();
+ if (poolImplementation_ == address(0) || poolImplementation_.code.length == 0) revert InvalidImplementation();
if (defaultOutcomeModerator_ == address(0)) revert ZeroAddress();
function setPoolImplementation(address newPoolImplementation) external onlyOwner {
- if (newPoolImplementation == address(0)) revert ZeroAddress();
+ if (newPoolImplementation == address(0) || newPoolImplementation.code.length == 0) revert InvalidImplementation();
address old = poolImplementation;
poolImplementation = newPoolImplementation;
emit PoolImplementationUpdated(old, newPoolImplementation);
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!