BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: high
Likelihood: low

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.

Support

FAQs

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

Give us feedback!