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

`createPool`/`initialize` never check the agreement's live registry state, allowing creation of permanently dead-on-arrival pools

Author Revealed upon completion

Root + Impact

Description

  • Normally, a ConfidencePool clone is created for an agreement that is still in its pre-attack
    staging window (NOT_DEPLOYED / NEW_DEPLOYMENT / ATTACK_REQUESTED), so stakers can deposit
    and the pool then organically observes the registry progressing through the rest of its
    lifecycle (UNDER_ATTACK → PROMOTION_REQUESTED → PRODUCTION/CORRUPTED).

  • Neither ConfidencePoolFactory.createPool nor ConfidencePool.initialize actually check the
    agreement's current AttackRegistry lifecycle state before creating/initializing a pool — they
    only check that the agreement itself is a genuine, factory-deployed agreement
    (isAgreementValid). A pool can therefore be created for an agreement that has already reached
    PROMOTION_REQUESTED, PRODUCTION, or CORRUPTED, all three of which _assertDepositsAllowed
    unconditionally rejects — so the pool can never accept a single stake() or contributeBonus()
    call for its entire lifetime.

// src/ConfidencePoolFactory.sol
function createPool(...) external whenNotPaused returns (address pool) {
if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
@> // no check on the agreement's AttackRegistry lifecycle state
...
}
// src/ConfidencePool.sol
function initialize(...) external initializer {
...
if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {
revert InvalidAgreement();
}
...
_replaceScope(accounts);
_transferOwnership(owner_);
@> // never calls _observePoolState() / reads getAgreementState() before finishing init
}
​
// _assertDepositsAllowed then permanently blocks every stake()/contributeBonus() call:
function _assertDepositsAllowed(IAttackRegistry.ContractState state) private pure {
if (
state == IAttackRegistry.ContractState.PROMOTION_REQUESTED
@> || state == IAttackRegistry.ContractState.PRODUCTION
@> || state == IAttackRegistry.ContractState.CORRUPTED
) {
revert StakingClosed();
}
}

Risk

Likelihood:

  • Occurs whenever createPool is called for an agreement whose AttackRegistry state has already
    progressed past ATTACK_REQUESTED (i.e. PROMOTION_REQUESTED, PRODUCTION, or CORRUPTED) —
    reachable any time pool creation lags behind the agreement's own lifecycle, e.g. a sponsor
    creating the pool late, or automation that fires off createPool without first checking
    getAgreementState.

  • Occurs regardless of createPool's other validations, since none of ZeroAddress,
    StakeTokenNotAllowed, ExpiryTooSoon, InvalidAgreement, and UnauthorizedCreator say
    anything about the registry's current lifecycle state — a call can pass every existing check and
    still produce a dead pool.

Impact:

  • The pool creator (only the agreement's own owner can call createPool for it) deploys a pool
    that can never accept a single stake or bonus contribution, with no revert or on-chain signal at
    creation time that anything is wrong.

  • createPool still succeeds and emits a normal-looking PoolCreated event, so integrators or
    automation watching that event (or getPoolsByAgreement) have no way to distinguish a live pool
    from a dead one without an out-of-band registry-state check of their own.

  • No staker funds are directly lost (staking reverts atomically before any token transfer), but the
    dead pool permanently occupies its deterministic Clones.cloneDeterministic address for that
    (agreement, index) slot.

Proof of Concept

function test_PoolCreatedAfterProductionIsPermanentlyUnusable() public {
// Agreement has already reached PRODUCTION before the pool is created.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
​
ConfidencePool implementation = new ConfidencePool();
ConfidencePool deadPool = ConfidencePool(Clones.clone(address(implementation)));
​
// initialize() succeeds — no revert, no signal that the pool is unusable.
deadPool.initialize(
agreement,
address(token),
address(safeHarborRegistry),
moderator,
block.timestamp + 31 days,
ONE,
recovery,
address(this),
_defaultScope()
);
​
// Every future staker is permanently blocked.
token.mint(alice, ONE);
vm.startPrank(alice);
token.approve(address(deadPool), ONE);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
deadPool.stake(ONE);
vm.stopPrank();
​
// Same for bonus contributions.
token.mint(bob, ONE);
vm.startPrank(bob);
token.approve(address(deadPool), ONE);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
deadPool.contributeBonus(ONE);
vm.stopPrank();
}

Recommended Mitigation

Read the agreement's live registry state during initialize and revert if it's already past
ATTACK_REQUESTED, instead of silently minting an unusable clone:

+ error AgreementAlreadyActive();
​
function initialize(
address agreement_,
address stakeToken_,
address safeHarborRegistry_,
address outcomeModerator_,
uint256 expiry_,
uint256 minStake_,
address recoveryAddress_,
address owner_,
address[] calldata accounts
) external initializer {
if (agreement_ == address(0)) revert ZeroAddress();
if (stakeToken_ == address(0)) revert ZeroAddress();
if (safeHarborRegistry_ == address(0)) revert ZeroAddress();
if (outcomeModerator_ == address(0)) revert ZeroAddress();
if (owner_ == address(0)) revert ZeroAddress();
if (recoveryAddress_ == address(0)) revert InvalidRecoveryAddress();
if (expiry_ < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (expiry_ > type(uint32).max) revert ExpiryTooFar();
if (minStake_ == 0) revert InvalidAmount();
if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {
revert InvalidAgreement();
}
+
+ IAttackRegistry.ContractState state = IAttackRegistry(
+ IBattleChainSafeHarborRegistry(safeHarborRegistry_).getAttackRegistry()
+ ).getAgreementState(agreement_);
+ if (
+ state != IAttackRegistry.ContractState.NOT_DEPLOYED
+ && state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
+ && state != IAttackRegistry.ContractState.ATTACK_REQUESTED
+ ) {
+ revert AgreementAlreadyActive();
+ }
​
agreement = agreement_;
stakeToken = IERC20(stakeToken_);
...

This keeps pool creation restricted to the window the rest of the design already assumes pools are
born in, and gives the creator an explicit, actionable revert instead of a silently dead clone.

Support

FAQs

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

Give us feedback!