Description
-
Normal behavior: once the pool locally observes a terminal registry state, the local risk-window end is sealed in riskWindowEnd. Withdrawals are intended to stay closed once risk has materialized, and the design documentation states that benign upstream state rewind cannot reopen withdrawals because of the pool's one-way local latches.
-
Specific issue: withdraw() only checks riskWindowStart and the current live registry state. When the pool first observes terminal CORRUPTED before any active-risk observation, riskWindowEnd is nonzero but riskWindowStart remains zero. A trusted registry cutover or state discontinuity can then make the live registry report a pre-risk state, reopening withdrawals even though the pool already recorded terminal CORRUPTED finality. Any withdrawn stake is excluded from the later CORRUPTED snapshot and reduces the attacker/recovery payout.
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();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
Risk
Likelihood:
-
A trusted DAO / Safe Harbor registry owner performs a canonical AttackRegistry cutover or migration that exposes a fresh registry view before per-agreement terminal state has been preserved.
-
A pool has already observed terminal CORRUPTED without first observing UNDER_ATTACK or PROMOTION_REQUESTED, leaving riskWindowEnd > 0 and riskWindowStart == 0.
Impact:
-
Stakers can withdraw principal after local terminal CORRUPTED finality has been recorded.
-
The later CORRUPTED snapshot is reduced, lowering the good-faith attacker bounty or bad-faith recovery payout by the withdrawn amount.
Proof of Concept
function test_F8_terminalFirstRewindReopensWithdrawals() external {
_stake(alice, 100 * ONE);
_stake(bob, 100 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
pool.pokeRiskWindow();
assertEq(pool.riskWindowStart(), 0, "terminal-first path has no active-risk latch");
assertGt(pool.riskWindowEnd(), 0, "terminal observation is locally recorded");
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.withdraw();
assertEq(token.balanceOf(alice) - aliceBefore, 100 * ONE, "withdraw reopens after terminal-first rewind");
assertEq(pool.totalEligibleStake(), 100 * ONE, "corrupted snapshot will be reduced");
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
assertEq(pool.snapshotTotalStaked(), 100 * ONE, "snapshot excludes withdrawn terminal-observer stake");
}
Command run:
forge test --match-path test/unit/KiraMigrationFindings.t.sol --match-test test_F8_terminalFirstRewindReopensWithdrawals -vv
Exact output:
Missing dependencies found. Installing now...
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' already exists and is not an empty directory.
fatal: clone of 'https://github.com/Cyfrin/battlechain-safe-harbor-contracts' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' failed
Failed to clone 'lib/battlechain-safe-harbor-contracts'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/forge-std' already exists and is not an empty directory.
fatal: clone of 'https://github.com/foundry-rs/forge-std' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/forge-std' failed
Failed to clone 'lib/forge-std'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts' already exists and is not an empty directory.
fatal: clone of 'https://github.com/OpenZeppelin/openzeppelin-contracts' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts' failed
Failed to clone 'lib/openzeppelin-contracts'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts-upgradeable' already exists and is not an empty directory.
fatal: clone of 'https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/openzeppelin-contracts-upgradeable' failed
Failed to clone 'lib/openzeppelin-contracts-upgradeable'. Retry scheduled
fatal: destination path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' already exists and is not an empty directory.
fatal: clone of 'https://github.com/Cyfrin/battlechain-safe-harbor-contracts' into submodule path '/home/nate/Cyfrin/BattleChainCP/Final/2026-07-bc-confidence-pools/lib/battlechain-safe-harbor-contracts' failed
Failed to clone 'lib/battlechain-safe-harbor-contracts' a second time, aborting
Warning: Your project has missing dependencies that could not be installed.
No files changed, compilation skipped
Ran 1 test for test/unit/KiraMigrationFindings.t.sol:KiraMigrationFindingsTest
[PASS] test_F8_terminalFirstRewindReopensWithdrawals() (gas: 530412)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.36ms (242.07µs CPU time)
Ran 1 test suite in 27.88ms (1.36ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
function withdraw() external nonReentrant {
if (outcome != PoolStates.Outcome.UNRESOLVED) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
if (
- riskWindowStart != 0
+ riskWindowStart != 0
+ || riskWindowEnd != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) {
revert WithdrawsDisabled();
}
}
Migration tooling should also prove per-agreement terminal-equivalent state before exposing a replacement AttackRegistry pointer.
