Root + Impact
Description
Under normal behavior, once a pool is resolved as SURVIVED or EXPIRED, sweepUnclaimedBonus() is intended to move only unreserved bonus funds to the recoveryAddress. The function intentionally does not set claimsStarted, so small dust donations cannot be used to prematurely close the moderator's pre-claim reflag window.
However, this creates an inconsistent finality boundary. sweepUnclaimedBonus() can move real pool funds while leaving claimsStarted == false, which allows the moderator to later reflag the same pool as good-faith CORRUPTED. Because flagOutcome() recomputes snapshotTotalBonus and bountyEntitlement from the current live accounting, the previously swept bonus is excluded from the attacker's later bounty entitlement.
As a result, a good-faith attacker can be underpaid after a valid moderator correction from SURVIVED to CORRUPTED.
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
...
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();
}
...
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
stakeToken.safeTransfer(recoveryAddress, amount);
}
Risk
Likelihood:
-
A moderator initially resolves a pool as SURVIVED while the registry has already reached a terminal CORRUPTED state, then later corrects the outcome to good-faith CORRUPTED.
-
During the still-open reflag window, any caller executes sweepUnclaimedBonus() and moves the unreserved bonus to recoveryAddress.
Impact:
-
The good-faith attacker bounty is recomputed from the reduced totalBonus, causing the attacker to receive less than the intended snapshotTotalStaked + snapshotTotalBonus.
-
Funds that should have been part of the good-faith corrupted bounty can be prematurely moved to recoveryAddress, creating an incorrect distribution between the attacker and recovery recipient.
Proof of Concept
function test_PoC_SweepBonusBeforeReflagReducesGoodFaithCorruptedBounty() public {
uint256 principal = 100 * ONE;
uint256 bonus = 50 * ONE;
_stake(alice, principal);
_contributeBonus(bob, bonus);
uint256 expectedFullBounty = pool.totalEligibleStake() + pool.totalBonus();
assertEq(expectedFullBounty, principal + bonus, "setup: full bounty should include principal plus bonus");
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
pool.pokeRiskWindow();
assertEq(pool.riskWindowStart(), 0, "setup: no local active-risk window observed");
assertGt(pool.riskWindowEnd(), 0, "setup: terminal state was observed");
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(pool.snapshotTotalStaked(), principal, "SURVIVED snapshot includes principal");
assertEq(pool.snapshotTotalBonus(), bonus, "SURVIVED snapshot includes bonus");
assertFalse(pool.claimsStarted(), "flagOutcome alone does not lock the reflag window");
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, bonus, "bonus was swept to recovery");
assertEq(pool.totalBonus(), 0, "live totalBonus was reduced by the sweep");
assertFalse(pool.claimsStarted(), "sweepUnclaimedBonus still leaves reflag window open");
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
assertEq(pool.bountyEntitlement(), principal, "bounty was recomputed without the swept bonus");
assertLt(
pool.bountyEntitlement(),
expectedFullBounty,
"BUG: pre-reflag bonus sweep reduces later good-faith CORRUPTED bounty"
);
}
Recommended Mitigation
The safest fix is to prevent bonus sweeps while the outcome can still be reflagged, or preserve the original resolution snapshot when later reflagging.
One minimal mitigation is to block sweepUnclaimedBonus() until the reflag window is closed:
function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
+ if (!claimsStarted) {
+ revert OutcomeAlreadySet();
+ }
uint256 reserved;
...
}
Alternatively, if the protocol wants to keep sweepUnclaimedBonus() callable before claims start, then flagOutcome() should not recompute corrupted bounty from reduced live accounting after a prior resolution snapshot:
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
+ uint256 previousSnapshotTotalStaked = snapshotTotalStaked;
+ uint256 previousSnapshotTotalBonus = snapshotTotalBonus;
+ bool wasPreviouslyResolved = outcome != PoolStates.Outcome.UNRESOLVED;
outcome = newOutcome;
goodFaith = goodFaith_;
attacker = attacker_;
- snapshotTotalStaked = totalEligibleStake;
- snapshotTotalBonus = totalBonus;
+ snapshotTotalStaked = wasPreviouslyResolved ? previousSnapshotTotalStaked : totalEligibleStake;
+ snapshotTotalBonus = wasPreviouslyResolved ? previousSnapshotTotalBonus : totalBonus;
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED
? snapshotTotalStaked + snapshotTotalBonus
: 0;
bountyEntitlement = willBeGoodFaithCorrupted
? snapshotTotalStaked + snapshotTotalBonus
: 0;
}
