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

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.

Updates

Lead Judging Commences

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

Missing riskWindowEnd rewind-latch in _assertDepositsAllowed lets late stakes/bonus theft via k=2 quadratic scoring

Impact - Medium Bonus pool only, principal always returns under SURVIVED/EXPIRED, but the quadratic advantage is unbounded and a tiny late stake can take nearly all of it. Likelihood - Low It needs a DAO registry migration to land inside the window where a pool is unresolved with riskWindowEnd already sealed, and the attacker still has to be watching. No attacker action can create the precondition.

Support

FAQs

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

Give us feedback!