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

sweepUnclaimedBonus() does not latch finality, so a post-sweep moderator correction to good-faith CORRUPTED re-snapshots a depleted totalBonus and shorts the named whitehat attacker the entire bonus

Author Revealed upon completion

Root + Impact

Root cause. sweepUnclaimedBonus() is the only value-moving function in ConfidencePool that does not set the claimsStarted finality latch, yet it irreversibly transfers the bonus to recoveryAddress and decrements the live totalBonus. The moderator's pre-claim re-flag window therefore stays open after value has already left the pool, and flagOutcome re-snapshots snapshotTotalBonus = totalBonus from the now-depleted value.

Impact. When riskWindowStart == 0, a permissionless sweepUnclaimedBonus() during a SURVIVED phase, followed by a legitimate moderator correction to good-faith CORRUPTED, re-snapshots snapshotTotalBonus = 0. The named whitehat attacker's bountyEntitlement then excludes the entire bonus: they receive only the staked principal, and the bonus is stranded at recoveryAddress. This directly contradicts docs/DESIGN.md §12 — "For good-faith CORRUPTED, bountyEntitlement … [is] snapshotTotalStaked + snapshotTotalBonus — the entire pool is the named attacker's bounty."

Description

sweepUnclaimedBonus() depletes totalBonus without latching finality:

// src/ConfidencePool.sol : sweepUnclaimedBonus()
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus; // irreversible depletion
}
// Intentionally does NOT set claimsStarted.
stakeToken.safeTransfer(recoveryAddress, amount);

Because claimsStarted stays false, the moderator can still re-flag, and flagOutcome re-snapshots from the depleted totalBonus:

// src/ConfidencePool.sol : flagOutcome()
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
...
snapshotTotalBonus = totalBonus; // reads 0 after the sweep
...
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;

Every other value-moving path (claimSurvived, claimExpired, claimCorrupted, claimAttackerBounty, sweepUnclaimedCorrupted) sets claimsStarted = true, which blocks re-flag. sweepUnclaimedBonus is the sole exception, and it is the exception that moves value while leaving the outcome re-flaggable.

This is distinct from the documented "no-risk-window CORRUPTED race" (DESIGN §5), which concerns claimExpired, not sweepUnclaimedBonus; that section does not address this path.

Risk

Likelihood: Low

  • riskWindowStart == 0 at resolution — no active-risk state was ever observed (nobody poked during UNDER_ATTACK/PROMOTION_REQUESTED, or the registry jumped straight to CORRUPTED). Common for pools with no active pokers.

  • The moderator first flags SURVIVED (breach read as out-of-scope — an allowed judgement, DESIGN §8), then corrects to good-faith CORRUPTED. The pre-claim re-flag window (DESIGN §4) exists precisely for such corrections.

  • Any address calls the permissionless sweepUnclaimedBonus() during the SURVIVED phase (legitimate under SURVIVED + riskWindowStart == 0, so it can happen innocently or be front-run to strip the bounty).

Impact: High

  • Total loss of the bonus — which can be the majority of the pool (protocol-readme: bonus liquidity is "economically required for rational staker participation") — to the intended whitehat recipient; the bonus is stranded at recoveryAddress.

Proof of Concept

Add test/poc/SweepReflagShort.poc.t.sol and run forge test --match-path 'test/poc/SweepReflagShort.poc.t.sol' -vv.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract SweepReflagShortPoC is BaseConfidencePoolTest {
function test_sweep_then_reflag_shorts_attacker_bounty() external {
// 1) stakers deposit pre-risk (registry NEW_DEPLOYMENT) so riskWindowStart never seals.
_stake(alice, 100 * ONE);
_stake(bob, 50 * ONE);
uint256 stakes = 150 * ONE;
​
// 2) sponsor funds a bonus
uint256 bonus = 40 * ONE;
_contributeBonus(makeAddr("sponsor"), bonus);
​
// 3) registry jumps straight to CORRUPTED without any active-risk observation.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
pool.pokeRiskWindow(); // seals riskWindowEnd (terminal) but NOT riskWindowStart
assertEq(pool.riskWindowStart(), 0, "precondition: riskWindowStart must be 0");
​
// 4) moderator first flags SURVIVED (reads the CORRUPTED breach as out-of-scope).
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
// 5) a permissionless griefer sweeps the (unclaimable) bonus to recovery.
vm.prank(makeAddr("griefer"));
pool.sweepUnclaimedBonus();
assertEq(pool.totalBonus(), 0, "totalBonus zeroed by sweep");
assertEq(pool.claimsStarted(), false, "sweep must not latch finality");
​
// 6) moderator legitimately corrects SURVIVED -> good-faith CORRUPTED, naming the whitehat.
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
// 7) attacker claims their full entitlement.
uint256 atkBefore = token.balanceOf(attacker);
vm.prank(attacker);
pool.claimAttackerBounty();
uint256 received = token.balanceOf(attacker) - atkBefore;
​
// shorted the full bonus: got only stakes.
assertEq(received, stakes, "attacker got only stakes (shorted the bonus)");
assertLt(received, stakes + bonus, "attacker shorted vs intended full pool");
}
​
function test_control_no_sweep_attacker_gets_full_pool() external {
_stake(alice, 100 * ONE);
_stake(bob, 50 * ONE);
uint256 stakes = 150 * ONE;
uint256 bonus = 40 * ONE;
_contributeBonus(makeAddr("sponsor"), bonus);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
pool.pokeRiskWindow();
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
// (no sweep here)
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
uint256 atkBefore = token.balanceOf(attacker);
vm.prank(attacker);
pool.claimAttackerBounty();
assertEq(token.balanceOf(attacker) - atkBefore, stakes + bonus, "control: attacker gets full pool");
}
}

Result (execution-grounded):

[PASS] test_sweep_then_reflag_shorts_attacker_bounty
attacker actually received : 150e18 <-- stakes only, shorted the full 40e18 bonus
[PASS] test_control_no_sweep_attacker_gets_full_pool
attacker receives : 190e18 <-- full pool when no sweep intervenes

The only difference between the two runs is the intervening sweepUnclaimedBonus(), which strips the entire bonus from the whitehat's realizable bounty.

Recommended Mitigation

Any one of:

  1. In the branch that actually removes bonus (riskWindowStart == 0), latch finality:

if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
+ // value has irreversibly left the pool; freeze the outcome against a later re-flag
+ if (!claimsStarted) claimsStarted = true;
}

  1. Gate sweepUnclaimedBonus behind finality (require claimsStarted, or a post-resolution delay) so no value leaves while the outcome is still re-flaggable.

  2. Base good-faith CORRUPTED bountyEntitlement on the pool's actual token balance at flag time rather than a totalBonus an intervening sweep can reduce.

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!