FoundrySolidityLayer 2
7.25 ETH
Submission Details
Severity: low
Valid

`sweepUnclaimedBonus()` Allows Value Movement Without Closing the Reflag Window, Reducing Later Good-Faith CORRUPTED Bounty

Author Revealed upon completion

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 {
// Re-flag allowed pre-claim so the moderator can fix a typo'd outcome / attacker before
// any participant locks in the wrong distribution.
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
...
outcome = newOutcome;
goodFaith = goodFaith_;
attacker = attacker_;
// @> Reflag recomputes snapshots from current live accounting.
snapshotTotalStaked = totalEligibleStake;
snapshotTotalBonus = totalBonus;
snapshotSumStakeTime = sumStakeTime;
snapshotSumStakeTimeSq = sumStakeTimeSq;
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED
? snapshotTotalStaked + snapshotTotalBonus
: 0;
// @> Good-faith bounty is recomputed from the reduced live totalBonus.
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) {
// @> Swept bonus is removed from live totalBonus.
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
// @> This intentionally does NOT set claimsStarted.
// @> Therefore the moderator can still reflag after real value has moved.
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");
// Terminal CORRUPTED observation seals riskWindowEnd but does not open riskWindowStart.
// With riskWindowStart == 0, SURVIVED/EXPIRED sweep treats bonus as unowed and sweepable.
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);
// Anyone can call sweepUnclaimedBonus and it intentionally does not set claimsStarted.
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");
// Moderator can still reflag to good-faith CORRUPTED, but the new bounty snapshot
// now uses the reduced live totalBonus.
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;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Validated
Assigned finding tags:

sweepUnclaimedBonus() omits claimsStarted latch, letting a moderator re-flag re-snapshot a drained totalBonus and short the CORRUPTED bounty

Impact – Medium Up to the entire bonus pool can be routed to the wrong party for good, with no on-chain way to unwind it, and in a stakerless pool the effect is total since bountyEntitlement computes to zero and claimAttackerBounty reverts outright. This impact is definitely not High: the pool stays solvent throughout with no principal ever at risk, and the money ends up at the sponsor's own recoveryAddress, which in most pools means a sponsor recovering a bonus they funded themselves. The whitehat's claim on that bonus also only exists because the moderator changed their mind after the fact. Likelihood – Low Four separate things have to coincide: nobody touches the pool for the whole active-risk window (or there are no stakers to begin with), the moderator flags SURVIVED and later reverses to CORRUPTED, that reversal is good-faith with a named attacker, and a sweep lands between the two flags. The first cuts against staker self-interest, since skipping the poke costs them their entire bonus share, and the second asks the moderator to overturn a scope judgement, which is a bigger deal than the typo fix DESIGN.md #4 offers the window for. The sweep itself I'd treat as near-certain once the rest holds, given it's permissionless and the sponsor has an obvious reason to make the call, but assembling the first three in one pool lifecycle is where this stays rare.

Support

FAQs

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

Give us feedback!