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

`createPool` accepts terminal agreements and deploys permanently non-depositable pools

Author Revealed upon completion

Root + Impact

Description

ConfidencePoolFactory.createPool() verifies that the supplied agreement is registered and owned
by the caller, but it does not verify the agreement's current AttackRegistry lifecycle state.
Likewise, ConfidencePool.initialize() checks isAgreementValid() without reading
getAgreementState().

// 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 validation of the agreement's current AttackRegistry state.
...
}

As a result, the agreement owner can successfully create a pool after the agreement has already
entered PRODUCTION or CORRUPTED. These states are terminal in the BattleChain registry, and
both are permanently rejected by _assertDepositsAllowed():

function _assertDepositsAllowed(IAttackRegistry.ContractState state) private pure {
if (
state == IAttackRegistry.ContractState.PROMOTION_REQUESTED
|| state == IAttackRegistry.ContractState.PRODUCTION
|| state == IAttackRegistry.ContractState.CORRUPTED
) {
revert StakingClosed();
}
}

The clone is therefore initialized, appended to getPoolsByAgreement(), and announced through a
normal PoolCreated event even though it can never accept a stake or bonus contribution. Every
subsequent deposit attempt reverts before transferring tokens.

This issue is limited to the terminal PRODUCTION and CORRUPTED states. Creation during
UNDER_ATTACK remains usable and is intentionally supported, while PROMOTION_REQUESTED can
return to UNDER_ATTACK if promotion is cancelled.

Risk

Likelihood

  • The condition is reached whenever the agreement owner creates a pool after the associated
    agreement has already finalized as PRODUCTION or CORRUPTED.

  • This can occur through delayed deployment or automation that validates agreement registration
    but does not separately query its lifecycle state.

  • All existing factory validations still pass because agreement validity and ownership do not
    imply that the agreement remains deposit-eligible.

Impact

  • The creator pays to deploy and initialize a clone that cannot perform its primary purpose.

  • The unusable address is permanently appended to the factory's pool-discovery list and emits the
    same PoolCreated event as a deposit-eligible pool, requiring integrators to perform an
    additional registry-state query before presenting it as active.

  • The deterministic address for that agreement/index is consumed, and the clone cannot be removed
    or reinitialized if creation was accidental.

  • No staker funds are lost because both deposit functions revert before token transfer; the impact
    is unnecessary deployment cost and permanent factory/discovery pollution.

Proof of Concept

Add the following imports and test to test/unit/ConfidencePoolFactory.t.sol:

import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
​
function test_PoolCreatedAfterProductionIsPermanentlyNonDepositable() external {
MockAttackRegistry attackRegistry = new MockAttackRegistry();
registry.setAttackRegistry(address(attackRegistry));
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
​
// Factory creation succeeds and registers the clone despite the terminal agreement state.
vm.prank(agreementOwner);
address created = factory.createPool(
address(agreement),
address(token),
block.timestamp + 31 days,
ONE,
recovery,
_scope()
);
​
assertEq(factory.poolCountByAgreement(address(agreement)), 1);
assertEq(factory.getPoolsByAgreement(address(agreement))[0], created);
​
ConfidencePool deadPool = ConfidencePool(created);
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
token.mint(alice, ONE);
vm.startPrank(alice);
token.approve(created, ONE);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
deadPool.stake(ONE);
vm.stopPrank();
​
token.mint(bob, ONE);
vm.startPrank(bob);
token.approve(created, ONE);
vm.expectRevert(IConfidencePool.StakingClosed.selector);
deadPool.contributeBonus(ONE);
vm.stopPrank();
}

The test shows that factory creation and registration succeed, while both ways of funding the pool
are permanently unavailable once the agreement is terminal.

Recommended Mitigation

Read the live registry state during initialization and reject terminal agreements before the clone
is committed to the factory's discovery list:

+ import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
​
+ error AgreementAlreadyTerminal();
​
function initialize(...) external initializer {
...
if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {
revert InvalidAgreement();
}
​
+ address attackRegistry =
+ IBattleChainSafeHarborRegistry(safeHarborRegistry_).getAttackRegistry();
+ if (attackRegistry == address(0)) revert InvalidAgreement();
+
+ IAttackRegistry.ContractState state =
+ IAttackRegistry(attackRegistry).getAgreementState(agreement_);
+ if (
+ state == IAttackRegistry.ContractState.PRODUCTION
+ || state == IAttackRegistry.ContractState.CORRUPTED
+ ) {
+ revert AgreementAlreadyTerminal();
+ }
​
...
}

Checking only terminal states preserves the intended ability to create and fund pools during
UNDER_ATTACK. It also permits creation during PROMOTION_REQUESTED, which may become usable
again if the promotion is cancelled. If the product intends to reject temporarily deposit-closed
pools as well, PROMOTION_REQUESTED can be included as an explicit policy choice.

Support

FAQs

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

Give us feedback!