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

Stakers can withdraw funds after the risk window has opened

Author Revealed upon completion

Root + Impact

Description

The withdraw() function is designed to allow stakers to exit the pool only during the pre-attack phase. According to the design documentation, withdrawals should be permanently disabled once the agreement enters an active-risk state (e.g., UNDER_ATTACK), which is marked by riskWindowStart being set.

The function's check for disabling withdrawals is: if (riskWindowStart != 0 || (state != ... && state != ... && state != ATTACK_REQUESTED)). The ATTACK_REQUESTED state is incorrectly included in the list of states where withdrawals are permitted. Per the documentation (DESIGN.md, section 9), ATTACK_REQUESTED is a pre-attack state where withdrawals should be allowed. However, the _isActiveRiskState helper function, which determines when to set riskWindowStart, does not include ATTACK_REQUESTED.

This creates a scenario where the agreement can be in the ATTACK_REQUESTED state, a user can call stake() (which does not set riskWindowStart), and then another user can call withdraw(), which will succeed because riskWindowStart is still 0 and ATTACK_REQUESTED is on the allowlist. This violates the design intent that any deposit made after the pre-attack staging phase should be locked until resolution.

// Root cause in the codebase with @> marks to highlight the relevant section
​
// In ConfidencePool.sol
function withdraw() external nonReentrant {
// ...
IAttackRegistry.ContractState state = _observePoolState();
// ...
if (
riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
@> && state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
revert WithdrawsDisabled();
}
// ...
}
​
function _isActiveRiskState(IAttackRegistry.ContractState s) internal pure returns (bool) {
@> return s == IAttackRegistry.ContractState.UNDER_ATTACK || s == IAttackRegistry.ContractState.PROMOTION_REQUESTED;
}
​

Risk

Likelihood: Medium

  • Reason 1

    • This occurs when the underlying agreement is in the ATTACK_REQUESTED state.

  • Reason 2

    • A user must call withdraw() during this specific state before any other interaction causes riskWindowStart to be set.

Impact: Medium

  • Impact 1

    • This allows a staker to escape their position after the pre-attack phase has ended, which undermines a core mechanic of the protocol.

  • Impact 2

    • t violates the documented lifecycle where withdrawals are permanently disabled from UNDER_ATTACK onward, creating a discrepancy between the code and the intended behavior described in DESIGN.md

Proof of Concept

// Add this test to a relevant test file, like `ConfidencePool.t.sol`
​
function test_PoC_WithdrawAfterAttackRequested() public {
// 1. Setup: A staker deposits funds into the pool.
uint256 stakeAmount = 1000e18;
vm.prank(staker);
pool.stake(stakeAmount);
assertEq(pool.eligibleStake(staker), stakeAmount);
​
// 2. Setup: The underlying agreement's state is moved to ATTACK_REQUESTED.
// This is a pre-attack state, but one where risk is imminent.
vm.prank(BATTLECHAIN_REGISTRY_OWNER);
attackRegistry.setAgreementState(address(agreement), IAttackRegistry.ContractState.ATTACK_REQUESTED);
​
// 3. Verification: A pool interaction at this stage does NOT set riskWindowStart.
// This is because _isActiveRiskState() does not consider ATTACK_REQUESTED an active risk state.
pool.pokeRiskWindow(); // This will revert, but we can also just check the state.
assertEq(pool.riskWindowStart(), 0, "riskWindowStart should NOT be set during ATTACK_REQUESTED");
​
// 4. Vulnerability: The staker calls withdraw().
// The call succeeds because riskWindowStart is 0 and the withdraw() function explicitly
// allows withdrawals during the ATTACK_REQUESTED state. This allows the staker to
// escape their position immediately before the agreement becomes attackable.
vm.prank(staker);
pool.withdraw();
​
// 5. Consequence: The staker has successfully withdrawn their funds, leaving the pool.
assertEq(pool.eligibleStake(staker), 0, "Staker's eligible stake should be zero after withdrawal");
assertEq(stakeToken.balanceOf(staker), stakeAmount, "Staker should have received their funds back");
​
log("PoC successful: Staker withdrew funds while agreement was in ATTACK_REQUESTED state.");
}
​

Recommended Mitigation

The _isActiveRiskState function should be updated to include ATTACK_REQUESTED as an active-risk state. This will ensure that any interaction with the pool while the agreement is in this state correctly sets riskWindowStart, which in turn will properly disable the withdraw function as intended.

Support

FAQs

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

Give us feedback!