BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Severity: low
Valid

sweepUnclaimedBonus moves the entire bonus without latching finality, so a later good-faith CORRUPTED re-flag re-snapshots a drained totalBonus and under-funds the named whitehat's bounty by the full bonus

Author Revealed upon completion

Description

claimsStarted is the finality latch: once value has moved, the moderator can no longer re-flag the outcome (DESIGN §4 — "finality is value-movement"). Every value-moving entrypoint sets it — except sweepUnclaimedBonus, which deliberately does not (to stop a 1-wei donation from griefing the re-flag window).

When riskWindowStart == 0 (no active-risk window was ever observed — nobody poked during UNDER_ATTACK, or the registry reached CORRUPTED fast), sweepUnclaimedBonus sweeps the entire bonus to recoveryAddress and decrements live totalBonus to zero — a major value movement that does not latch claimsStarted. The moderator can then still make the normal §4 correction from SURVIVED to good-faith CORRUPTED (naming the whitehat who legally rescued the in-scope contract). That re-flag re-snapshots snapshotTotalBonus = totalBonus, which is now zero, so bountyEntitlement = snapshotTotalStaked — the whitehat is entitled to principal only. Per DESIGN §12 the whole pool (principal + bonus) is the whitehat's bounty; the bonus has been captured by the sponsor's recoveryAddress.

// sweepUnclaimedBonus: with riskWindowStart == 0 the bonus is NOT reserved -> the ENTIRE bonus is swept
uint256 reserved;
if (totalEligibleStake != 0) {
reserved = totalEligibleStake;
if (riskWindowStart != 0) {
reserved += snapshotTotalBonus - claimedBonus; // <- skipped when riskWindowStart == 0
}
}
uint256 amount = freeBalance > reserved ? freeBalance - reserved : 0; // = the whole bonus
// ...
if (totalEligibleStake == 0 || riskWindowStart == 0) {
@> totalBonus -= amount <= totalBonus ? amount : totalBonus; // drains live totalBonus to 0
}
// "Intentionally does NOT set claimsStarted." -> a MAJOR value movement without latching finality
@> stakeToken.safeTransfer(recoveryAddress, amount);
​
// flagOutcome (the later good-faith CORRUPTED re-flag) re-snapshots the DRAINED totalBonus:
snapshotTotalBonus = totalBonus; // == 0
@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0; // = principal only

The code comment at the decrement even states it drops totalBonus "keeping the accounting honest for any later re-snapshot" — it anticipates the re-snapshot but does not account for it under-funding the whitehat.

Risk

Likelihood:

  • Occurs when riskWindowStart == 0 (the pool never observed the active-risk window — realistic when the whitehat operated at the agreement level and never poked this third-party pool, or the registry reached CORRUPTED quickly).

  • Occurs when the moderator makes the NORMAL §4 correction — an initial SURVIVED judgement (breach looked out-of-scope) later corrected to good-faith CORRUPTED (determined in-scope). This is the exact use the correction window is designed for, seconds apart — no 180-day wait.

  • Occurs while the sponsor (or anyone — the sweep is permissionless) calls sweepUnclaimedBonus in the gap; the sponsor is economically incentivised (they own recoveryAddress).

Impact:

  • The moderator-named whitehat, who performed a legal Safe-Harbor rescue, loses their entire bonus — which can be the majority of the reward the sponsor put up to attract whitehats. It is captured by the sponsor's recoveryAddress.

  • The stated DESIGN §4 invariant ("finality is value-movement; claimsStarted latches") is violated: the whole bonus leaves the pool without latching, and a subsequent re-flag silently under-funds the bounty.

Proof of Concept

test/audit/RVS1.t.sol — the leak plus a control proving it is specific to riskWindowStart == 0:

function testRVS1_sweepBeforeReflagRobsWhitehatBonus() external {
uint256 P = 100 * ONE; uint256 B = 400 * ONE; // principal + bonus (the whitehat's earned reward)
attackRegistry.setAgreementState(IAttackRegistry.ContractState.ATTACK_REQUESTED);
_stake(alice, P); _contributeBonus(carol, B);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK); // nobody pokes
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0)); // (1) initial judgement
assertEq(uint256(pool.riskWindowStart()), 0);
​
pool.sweepUnclaimedBonus(); // (2) permissionless: bonus -> recovery
assertEq(pool.totalBonus(), 0);
assertEq(pool.claimsStarted(), false); // NOT latched (the bug)
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat); // (3) normal §4 correction
assertEq(pool.bountyEntitlement(), P); // BUG: should be P + B
​
vm.prank(whitehat);
pool.claimAttackerBounty();
assertEq(token.balanceOf(whitehat), P); // whitehat got principal ONLY; bonus lost
}
// Control (also passes): with a poke (riskWindowStart != 0), sweepUnclaimedBonus reserves the bonus and
// reverts; the re-flag yields bountyEntitlement == P + B and the whitehat is made whole.

Run: forge test --match-contract RVS1Test -vv

Recommended Mitigation

Latch finality in the branch that moves genuinely-reserved bonus (the dust-donation griefing case goes through the other branch, which still does not latch):

if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
+ // This branch moves real reserved bonus (not just dust). Latch finality so a later
+ // good-faith CORRUPTED re-flag cannot re-snapshot the drained totalBonus and under-fund the whitehat.
+ if (!claimsStarted) claimsStarted = true;
}

Alternatively, on a good-faith CORRUPTED re-flag, reserve the already-swept bonus rather than re-snapshotting the drained totalBonus.

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!