Root + Impact
Description
The single claimsStarted flag represents both economic reliance and mechanical resolution finality, but different entrypoints latch it under different conditions. A value-moving bonus sweep leaves re-flagging open, a zero-value expiry resolver closes it, and zero-payout bounty calls leave it open, making correction finality depend on which permissionless transaction executes first.
The moderator may correct an already-flagged outcome until claimsStarted becomes true. The stated rationale is to allow typo correction before anyone relies on the old distribution, then make the first value movement final.
The implementation follows that rule for ordinary claims and nonzero bounty payments, but not uniformly. sweepUnclaimedBonus() can transfer tracked bonus and reduce totalBonus without setting the latch. Conversely, the first claimExpired() sets it during mechanical resolution even when the caller owns no stake and receives nothing. These choices may each be defensible in isolation, but one boolean no longer communicates or enforces a consistent finality invariant.
src/ConfidencePool.sol
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
emit OutcomeFlagged(msg.sender, newOutcome, goodFaith_, attacker_);
}
function claimSurvived() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED) revert OutcomeNotSet();
if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(msg.sender, payout);
emit ClaimSurvived(msg.sender, userEligible, bonusShare);
}
function claimAttackerBounty() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (payout > 0) {
corruptedReserve -= payout;
if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(attacker, payout);
}
emit AttackerBountyClaimed(attacker, payout, newBountyClaimed, bountyEntitlement);
}
function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
function claimExpired() external nonReentrant {
if (block.timestamp < expiry) revert PoolNotExpired();
if (outcome == PoolStates.Outcome.UNRESOLVED) {
claimsStarted = true;
}
uint256 userEligible = eligibleStake[msg.sender];
if (userEligible == 0) {
return;
}
}
As a result, transaction ordering can decide whether the moderator can correct the outcome. A beneficiary can close the window with a real claim before a pending correction; a permissionless bonus sweep can move tracked value yet still allow a later outcome rewrite; and any zero-stake expiry caller can finalize the outcome without economic reliance. The current behavior is partly documented as intentional, so this is a policy/finality inconsistency rather than an access-control bypass.
Risk
Likelihood:
-
The issue matters when an initial outcome needs correction and a claim, sweep, bounty call, or expiry resolution is executed before the moderator's corrective transaction.
-
Exploitation is timing-dependent and usually requires observing a pending correction or a delayed moderator, while the registry state must also permit the desired replacement outcome.
Impact:
-
Closing the window first can permanently preserve an erroneous outcome and its complete stake/bonus distribution.
-
Leaving the window open after tracked value moves lets a later re-flag snapshot reduced accounting, underfund a corrected bounty, or otherwise produce a distribution inconsistent with pre-sweep expectations.
Proof of Concept
Create test/audit/CP034ClaimsStartedFinalityEdges.t.sol with the following contents:
pragma solidity 0.8.26;
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
contract CP034ClaimsStartedFinalityEdgesTest is BaseConfidencePoolTest {
function test_BonusSweepMovesTrackedValueButLeavesReflagWindowOpen() external {
_contributeBonus(carol, 50 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, 50 * ONE);
assertEq(pool.totalBonus(), 0);
assertFalse(pool.claimsStarted(), "tracked bonus moved without closing correction");
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED));
}
function test_ZeroStakeClaimExpiredClosesReflagWindowWithoutPayout() external {
_stake(alice, 100 * ONE);
vm.warp(pool.expiry());
uint256 daveBefore = token.balanceOf(dave);
vm.prank(dave);
pool.claimExpired();
assertEq(token.balanceOf(dave), daveBefore, "resolver receives no payout");
assertTrue(pool.claimsStarted(), "mechanical resolution closes correction without value movement");
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
vm.expectRevert(IConfidencePool.OutcomeAlreadySet.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
}
function test_ZeroPayoutBountyLeavesReflagWindowOpen() external {
_stake(alice, 100 * ONE);
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
deal(address(token), address(pool), 0);
vm.prank(attacker);
pool.claimAttackerBounty();
assertEq(pool.bountyClaimed(), 0);
assertFalse(pool.claimsStarted(), "zero payout does not latch finality");
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.SURVIVED));
}
function test_FirstValueMovingClaimClosesReflagWindow() external {
_stake(alice, 100 * ONE);
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
vm.prank(alice);
pool.claimSurvived();
assertTrue(pool.claimsStarted());
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
vm.expectRevert(IConfidencePool.OutcomeAlreadySet.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
}
}
Run the PoC from the repository root:
-
Execute forge test --offline --match-path test/audit/CP034ClaimsStartedFinalityEdges.t.sol -vv.
-
Confirm that all four tests pass and demonstrate the open/closed re-flag states after tracked bonus movement, zero-value mechanical resolution, zero-payout bounty execution, and an ordinary value-moving claim.
Recommended Mitigation
Separate mechanical resolution finality from economic-reliance finality. Latch economic action only when protocol-accounted value moves, so a direct 1-wei donation cannot grief correction, but sweeping tracked bonus does close the window.
src/ConfidencePool.sol
-bool public claimsStarted;
+bool public mechanicalResolutionFinal;
+bool public economicActionStarted;
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
- if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
+ if (
+ outcome != PoolStates.Outcome.UNRESOLVED
+ && (mechanicalResolutionFinal || economicActionStarted)
+ ) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
// ... validation and snapshot logic omitted ...
emit OutcomeFlagged(msg.sender, newOutcome, goodFaith_, attacker_);
}
function sweepUnclaimedBonus() external nonReentrant {
// ... reserve and amount calculation omitted ...
+ uint256 trackedBonusSwept;
if (totalEligibleStake == 0 || riskWindowStart == 0) {
- totalBonus -= amount <= totalBonus ? amount : totalBonus;
+ trackedBonusSwept = amount <= totalBonus ? amount : totalBonus;
+ totalBonus -= trackedBonusSwept;
}
+ if (trackedBonusSwept != 0) economicActionStarted = true;
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
function claimExpired() external nonReentrant {
if (block.timestamp < expiry) revert PoolNotExpired();
// ... mechanical resolution omitted ...
if (outcome == PoolStates.Outcome.UNRESOLVED) {
// ... snapshot and outcome selection omitted ...
- claimsStarted = true;
+ mechanicalResolutionFinal = true;
}
// ... optional caller payout omitted ...
}
Update ordinary claims, corrupted sweeps, and nonzero bounty transfers to set economicActionStarted; leave genuine zero-payout calls unlatching. If the intended policy is instead “no correction after any successful post-resolution call,” encode that rule explicitly and accept the 1-wei donation grief tradeoff.
