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

[M-01] CEI Violation in createPool() - _poolsByAgreement written after external initialize() call

Author Revealed upon completion

Root + Impact

Description

ConfidencePoolFactory.createPool() deploys a new pool clone, initializes it, then records it in _poolsByAgreement. The mapping tracks all pools per agreement and is queried via getPoolsByAgreement() and poolCountByAgreement().

The state write _poolsByAgreement[agreement].push(pool) (line 103) happens after the external call IConfidencePool(pool).initialize(...) (line 90–101). Inside initialize(), _replaceScope() calls IAgreement(agreement).isContractInScope(account) for every scope account, transferring execution control to the caller-supplied agreement contract. During this entire window, _poolsByAgreement has not yet been updated, violating Checks-Effects-Interactions.

// ConfidencePoolFactory.sol
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
pool = Clones.cloneDeterministic(poolImplementation, salt);
​
// @> External call before state update agreement.isContractInScope() is called inside here
IConfidencePool(pool).initialize(
agreement, stakeToken, address(safeHarborRegistry),
defaultOutcomeModerator, expiry, minStake, recoveryAddress, msg.sender, accounts
);
​
// @> State written after the external call _poolsByAgreement is stale during initialize()
_poolsByAgreement[agreement].push(pool);

Risk

Likelihood:

  • A malicious or compromised agreement contract must override isContractInScope() to trigger a reentrant call. A reenter with the same agreement is self-neutralizing (same salt → same address → CREATE2 fails), limiting direct exploitation.

  • Exploitation becomes realistic only if a future factory method reads poolCountByAgreement and makes security decisions based on it during the same transaction.

Impact:

  • During the initialize() execution window, poolCountByAgreement(agreement) returns a stale value (one fewer than reality). Any on-chain logic or integration that reads this during the window operates on incorrect state.

  • Future factory upgrades that add logic relying on _poolsByAgreement correctness within the same call could introduce exploitable reentrancy paths, as the invariant is silently broken today.

Proof of Concept

The attack requires a malicious agreement contract whose isContractInScope() function re-enters the factory. When createPool() calls pool.initialize(), control passes to _replaceScope(), which calls agreement.isContractInScope(account) for each scope account. At this exact point, _poolsByAgreement[agreement] has not yet been updated, so its length is stale. A reentrant call to createPool() with a different agreement address will succeed because the salt for the new pool uses a different key. The original agreement's pool count remains wrong until the outermost call completes.

// 1. Attacker deploys MaliciousAgreement where isContractInScope() reenters the factory.
// 2. Attacker calls factory.createPool(maliciousAgreement, ...).
// 3. Inside pool.initialize() → _replaceScope() → maliciousAgreement.isContractInScope():
// - factory._poolsByAgreement[maliciousAgreement].length is still 0 (stale)
// - Reentrant createPool(differentAgreement, ...) executes and succeeds
// - A second pool is created with _poolsByAgreement[differentAgreement] still at 0
// 4. The outer createPool() resumes and pushes the first pool into _poolsByAgreement.
// Result: both pool mappings were computed using stale lengths during the reentrancy window.

Recommended Mitigation

Capture the current pool index before deploying, push to the mapping immediately after deployment (before any external call), then initialize:

+ uint256 poolIndex = _poolsByAgreement[agreement].length;
- bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
+ bytes32 salt = keccak256(abi.encode(agreement, poolIndex));
pool = Clones.cloneDeterministic(poolImplementation, salt);
​
+ _poolsByAgreement[agreement].push(pool);
​
IConfidencePool(pool).initialize(
agreement, stakeToken, address(safeHarborRegistry),
defaultOutcomeModerator, expiry, minStake, recoveryAddress, msg.sender, accounts
);
​
- _poolsByAgreement[agreement].push(pool);

Support

FAQs

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

Give us feedback!