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

A reverting withdrawal cannot persist the risk-window latch

Author Revealed upon completion

Root + Impact

Description

When withdraw() is the first pool interaction to observe active risk, it writes riskWindowStart and then reverts, rolling the supposedly permanent latch back and allowing a later registry rewind to reopen withdrawal.

Withdrawals are intended to be available only in NOT_DEPLOYED, NEW_DEPLOYMENT, and ATTACK_REQUESTED, then permanently disabled from UNDER_ATTACK onward. The pool deliberately supplements its live registry check with the one-way riskWindowStart latch so a legitimate upstream registry replacement or temporary state rewind cannot reopen the exit.

withdraw() first calls _observePoolState(). When the live state is UNDER_ATTACK or PROMOTION_REQUESTED, the observer sets riskWindowStart, resets the global bonus moments, and emits the risk-window event. The withdrawal gate then sees the newly written latch and reverts WithdrawsDisabled.

EVM reverts are atomic. The revert unwinds not only the withdrawal, but also riskWindowStart, scopeLocked, the moment resets, and their events. Consequently, the withdrawal that first discovers active risk cannot preserve the historical fact needed to make its rejection permanent.

If no other successful entrypoint calls _observePoolState() before the trusted registry pointer is replaced or migrated to an instance temporarily reporting a pre-risk state, the next withdrawal sees both:

riskWindowStart == 0
live state == NEW_DEPLOYMENT (or another allowed pre-risk state)

It therefore succeeds and deletes the staker's principal from pool accounting. When the replacement registry catches up and the moderator resolves CORRUPTED, the escaped stake is absent from snapshotTotalStaked, corruptedReserve, and any attacker bounty. The staker avoids the distribution they were intended to be irrevocably committed to from the first active-risk observation onward.

src/ConfidencePool.sol
​
function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState(); // @> Tentatively writes the latch.
if (
riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
revert WithdrawsDisabled(); // @> Rolls every observation write back.
}
​
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
// ... accounting omitted ...
​
stakeToken.safeTransfer(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
}
​
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
// ... scope lock omitted ...
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart(); // @> This state change cannot outlive the caller's revert.
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
​
function _markRiskWindowStart() internal {
uint256 t = block.timestamp;
if (t > expiry) t = expiry;
riskWindowStart = uint32(t);
sumStakeTime = totalEligibleStake * t;
sumStakeTimeSq = totalEligibleStake * t * t;
emit RiskWindowStarted(t);
}

The existing permissionless pokeRiskWindow() reduces exposure but does not enforce the invariant. It helps only when a separate successful transaction is included before the rewind. The natural first interaction can be the staker's failed withdrawal itself, and the protocol does not guarantee a keeper will persist the window first.

Moving the latch assignment earlier inside withdraw() cannot fix the issue. Any write made anywhere in the same call frame is reverted when withdraw() reverts.

Risk

Likelihood:

  • BattleChain's ordinary registry lifecycle does not transition from active risk back to a pre-risk state. Exploitation requires a registry replacement, migration, reset, or equivalent trusted infrastructure event to present a pre-risk state while the pool is still unresolved.

  • withdraw() must be the first pool interaction that observes active risk. Any successful pokeRiskWindow(), active-risk stake, bonus contribution, or other non-reverting observer persists riskWindowStart and closes this path.

  • Once those conditions occur, any staker can exploit the reopened exit without privilege, unusual token behavior, or control over outcome resolution.

Impact:

  • A staker can recover their entire eligible principal after the documented point at which that principal became permanently committed to resolution.

  • The escaped amount is deleted from totalEligibleStake and omitted from the later outcome snapshot. This reduces corruptedReserve for bad-faith corruption or bountyEntitlement for good-faith corruption by the full withdrawn amount.

  • The PoC demonstrates Alice escaping with half of the pool while the later CORRUPTED recovery sweep receives only Bob's remaining half.

Proof of Concept

Create test/audit/CP004WithdrawalRiskLatchRollback.t.sol with the following contents:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
​
contract CP004WithdrawalRiskLatchRollbackTest is BaseConfidencePoolTest {
function test_RevertingWithdrawalLetsRewoundRegistryReopenExitBeforeCorruption() external {
uint256 amount = 100 * ONE;
_stake(alice, amount);
_stake(bob, amount);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
​
// withdraw() tentatively opens riskWindowStart, then its own guard reverts the complete
// transaction. The supposedly one-way local witness therefore remains unset.
vm.prank(alice);
vm.expectRevert(IConfidencePool.WithdrawsDisabled.selector);
pool.withdraw();
​
assertEq(pool.riskWindowStart(), 0, "the reverting observer cannot persist the risk latch");
assertFalse(pool.scopeLocked(), "all observation side effects were rolled back");
​
// A legitimate registry replacement presents a pre-risk state before importing history.
MockAttackRegistry replacementRegistry = new MockAttackRegistry();
replacementRegistry.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
safeHarborRegistry.setAttackRegistry(address(replacementRegistry));
​
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.withdraw();
​
assertEq(token.balanceOf(alice) - aliceBefore, amount, "Alice escapes with all principal");
assertEq(pool.eligibleStake(alice), 0);
assertEq(pool.totalEligibleStake(), amount, "only Bob remains exposed");
​
// Once the replacement registry catches up, CORRUPTED resolution snapshots only Bob's
// stake; Alice's escaped amount is absent from the recovery sweep.
replacementRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
​
assertEq(pool.snapshotTotalStaked(), amount, "the escaped stake is missing from the snapshot");
​
uint256 recoveryBefore = token.balanceOf(recovery);
pool.claimCorrupted();
​
assertEq(token.balanceOf(recovery) - recoveryBefore, amount, "recovery receives only Bob's stake");
assertEq(token.balanceOf(alice), amount, "Alice avoids the corrupted distribution");
assertEq(token.balanceOf(address(pool)), 0);
}
}

Run the PoC from the repository root:

  1. Execute forge test --offline --match-path test/audit/CP004WithdrawalRiskLatchRollback.t.sol -vv.

  2. Confirm that the test passes and the suite reports one test passed with zero failures.

Recommended Mitigation

A pool-local fix must allow the first disallowed withdrawal call to complete successfully without transferring principal, so its monotonic lock can persist. Make this outcome explicit through both a return value and an event; later attempts can retain the existing revert behavior.

src/interfaces/IConfidencePool.sol
​
+event WithdrawalsPermanentlyLocked(uint256 timestamp);
​
-function withdraw() external;
+/// @return withdrawn True only when principal was transferred; false when this call first
+/// observes risk and permanently locks withdrawals without transferring principal.
+function withdraw() external returns (bool withdrawn);
​
src/ConfidencePool.sol
​
+bool public withdrawalsLocked;
​
-function withdraw() external nonReentrant {
+function withdraw() external nonReentrant returns (bool withdrawn) {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
+ if (withdrawalsLocked) revert WithdrawsDisabled();
IAttackRegistry.ContractState state = _observePoolState();
- if (
+ bool mustLock =
riskWindowStart != 0
+ || riskWindowEnd != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
- && state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
- ) {
- revert WithdrawsDisabled();
+ && state != IAttackRegistry.ContractState.ATTACK_REQUESTED);
+ if (mustLock) {
+ withdrawalsLocked = true;
+ emit WithdrawalsPermanentlyLocked(block.timestamp);
+ return false;
}
​
uint256 amount = eligibleStake[msg.sender];
if (amount == 0) revert InvalidAmount();
// ... unchanged accounting ...
​
stakeToken.safeTransfer(msg.sender, amount);
emit Withdrawn(msg.sender, amount);
+ return true;
}

Checking riskWindowEnd as well prevents a persisted terminal observation with no active-risk start from being ignored after a rewind, while keeping the bonus-accounting meaning of riskWindowStart unchanged.

All callers and frontends must handle the new false result/event as a rejected withdrawal. If revert-on-rejection semantics must be preserved for every call, a self-contained pool fix is impossible: the trusted registry or migration layer must instead expose an irreversible everReachedActiveRisk/highest-state witness that is preserved across registry replacements. withdraw() can query that durable history and revert without relying on a write made in its own reverting transaction.

As a weaker operational mitigation, require every registry migration to successfully call pokeRiskWindow() for every unresolved pool before changing the pointer. This introduces a keeper-completeness dependency and should not be described as autonomous rewind protection.

Support

FAQs

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

Give us feedback!