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

Factory accepts a codeless pool implementation and records unusable clones

Author Revealed upon completion

Root + Impact

The factory rejects a zero poolImplementation but accepts any nonzero address, including one with no runtime code. createPool clones that target, the clone's initialize delegatecall returns success with empty data, and the factory records the clone in _poolsByAgreement and emits PoolCreated. The recorded address is not a working pool: calls through it currently succeed as empty no-ops, leaving an unusable entry in the per-Agreement discovery list.

Description

  • The zero-address checks prevent an explicitly null implementation configuration but do not verify that the selected address has runtime code. createPool assumes the clone is a working ConfidencePool once the no-return-value initialize call completes.

  • Both factory initialize and setPoolImplementation accept any nonzero address. A no-code target, such as an EOA or undeployed address, can still be cloned through Clones.cloneDeterministic. The proxy's delegatecall to that target reports success with empty returndata, which satisfies the ABI call because initialize has no return value. The factory therefore appends the clone and emits PoolCreated. The factory cannot repoint that clone's immutable implementation target, and the current factory API provides no way to remove the recorded address.

// src/ConfidencePoolFactory.sol::setPoolImplementation — nonzero is the only check (same in initialize)
@> if (newPoolImplementation == address(0)) revert ZeroAddress();
​
// src/ConfidencePoolFactory.sol::createPool — no implementation-code or initialization postcondition check
pool = Clones.cloneDeterministic(poolImplementation, salt);
@> IConfidencePool(pool).initialize(...); // no-code delegatecall reports success with empty data
_poolsByAgreement[agreement].push(pool);
emit PoolCreated(...);

Risk

Likelihood:

  • Occurs when the factory owner sets poolImplementation to a nonzero address that lacks runtime code (a mistyped or EOA address, or an implementation not yet deployed). Every pool created until the value is corrected is a phantom.

  • Requires a factory-owner configuration error; no unprivileged actor can trigger it.

Likelihood is Low.

Impact:

  • The per-Agreement discovery list and PoolCreated events contain unusable addresses that currently answer calls as empty no-ops. The clone's target cannot be changed, and the current factory API cannot remove the entry; a factory upgrade would be required to add a cleanup path.

  • No user tokens move, existing pools are unaffected, and a valid replacement can be created at the next per-Agreement index after correcting the implementation. Because pool lists permit multiple entries and are curated off-chain, the immediate impact is limited to configuration and discovery pollution.

Impact is Low.

Proof of Concept

poc/NoCodeImplementationPoc.t.sol passes with:

forge test --contracts audit/findings/low/poc \
--match-path audit/findings/low/poc/NoCodeImplementationPoc.t.sol \
--match-test test_poc_noCodeImplementationRecordsPhantomPool -vv
function test_poc_noCodeImplementationRecordsPhantomPool() public {
address noCodeImplementation = makeAddr("noCodeImplementation");
assertEq(noCodeImplementation.code.length, 0);
factory.setPoolImplementation(noCodeImplementation);
​
vm.prank(agreementOwner);
address phantom =
factory.createPool(address(agreement), address(token), block.timestamp + 31 days, ONE, recovery, _scope());
​
address[] memory pools = factory.getPoolsByAgreement(address(agreement));
assertEq(pools.length, 1);
assertEq(pools[0], phantom);
assertGt(phantom.code.length, 0, "the EIP-1167 proxy itself has code");
​
// The proxy delegates to an address with no code. EVM DELEGATECALL reports success with
// empty returndata, so calls appear successful while no pool state or accounting changes.
(bool success, bytes memory returndata) = phantom.call(abi.encodeCall(IConfidencePool.stake, (ONE)));
assertTrue(success);
assertEq(returndata.length, 0);
​
(success, returndata) = phantom.staticcall(abi.encodeWithSignature("owner()"));
assertTrue(success);
assertEq(returndata.length, 0);
}

Recommended Mitigation

Require runtime code in both implementation-configuration paths (initialize and setPoolImplementation):

// src/ConfidencePoolFactory.sol::setPoolImplementation (mirror in initialize)
- if (newPoolImplementation == address(0)) revert ZeroAddress();
+ if (newPoolImplementation == address(0) || newPoolImplementation.code.length == 0) {
+ revert InvalidPoolImplementation();
+ }

Support

FAQs

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

Give us feedback!