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

`sweepUnclaimedBonus` between a SURVIVED flag and a good-faith CORRUPTED re-flag permanently strips the whitehat's bonus

Author Revealed upon completion

Root + Impact

Root cause: sweepUnclaimedBonus sends the whole bonus to the sponsor's recoveryAddress and zeroes totalBonus without setting claimsStarted — so the moderator's re-flag window stays open — and flagOutcome then re-reads the now-zero totalBonus into snapshotTotalBonus on the CORRUPTED re-flag.

Impact: On a good-faith CORRUPTED outcome the whitehat is entitled to the entire pool (snapshotTotalStaked + snapshotTotalBonus, §12) but gets principal only; the bonus is permanently redirected to the sponsor. PoC: 40 of 140 tokens (28.6% of the pool) to the wrong party.

Description

The moderator may re-flag an outcome until the first claim (flagOutcome gates on claimsStarted) — a deliberate pre-claim correction window (§4). The bug is that a sweepUnclaimedBonus in that window moves real value out but leaves the window open. When riskWindowStart == 0, sweep reserves only principal, sends the full bonus to recoveryAddress, zeroes totalBonus, and does not set claimsStarted:

// sweepUnclaimedBonus — the riskWindowStart==0 branch reserves principal only, bonus fully swept:
if (riskWindowStart != 0) reserved += snapshotTotalBonus - claimedBonus; // skipped when ==0
...
if (totalEligibleStake == 0 || riskWindowStart == 0)
totalBonus -= amount <= totalBonus ? amount : totalBonus; // totalBonus -> 0
// claimsStarted NOT set -> moderator re-flag window stays open

flagOutcome then re-snapshots the drained bonus on the CORRUPTED re-flag:

snapshotTotalBonus = totalBonus; // now 0
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0; // stake + 0

This is not documented behavior: §5 says a CORRUPTED outcome sweeps "the pool whole, bonus included" (this does the opposite), and §4's no-claimsStarted rule is documented only for the dust-donation case, not a material bonus sweep.

Risk

Likelihood: Low — needs the moderator to flag SURVIVED then correct to good-faith CORRUPTED, with riskWindowStart == 0 and a sweep in between. The sponsor controls recoveryAddress, so has a direct incentive to be the sweeper.

Impact: Medium — a supported correction produces a wrong, permanent payout: the identical terminal state pays 140 via a direct CORRUPTED flag but only 100 here. Not High because no principal is destroyed and the bonus lands at the sponsor-controlled recoveryAddress (not stolen by a third party) — hence an honest Low floor, Medium arguable.

Proof of Concept

Executed against the contest suite with forge test. Both a control (the correct good-faith CORRUPTED payout) and the bug pass, so the two payouts are directly comparable on the same terminal state.

Where to place the file: create test/PoC_BountyUnderstatement.t.sol (any path under test/ works; it extends the contest's own test/helpers/BaseConfidencePoolTest.sol, so no extra setup is needed).

Run command:

forge test --match-contract PoC_BountyUnderstatement -vv

Full PoC (complete file):

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// PoC: sweepUnclaimedBonus (riskWindowStart==0) between a SURVIVED flag and a good-faith CORRUPTED
/// re-flag permanently understates the whitehat's bountyEntitlement by the whole bonus.
contract PoC_BountyUnderstatement is BaseConfidencePoolTest {
// CONTROL: correct good-faith CORRUPTED where the whitehat receives stake + bonus (full pool).
function test_control_whitehatGetsFullPool() external {
_stake(alice, 100 * ONE); // principal S = 100
_contributeBonus(carol, 40 * ONE); // bonus B = 40
​
// Registry goes CORRUPTED directly (no active-risk observed -> riskWindowStart == 0).
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker); // good-faith, correct outcome
​
assertEq(pool.bountyEntitlement(), 140 * ONE, "control: bounty = stake + bonus");
​
vm.prank(attacker);
pool.claimAttackerBounty();
assertEq(token.balanceOf(attacker), 140 * ONE, "control: whitehat gets full pool 140");
}
​
// BUG: an interleaved sweepUnclaimedBonus between a SURVIVED flag and the CORRUPTED re-flag
// strips the bonus to recoveryAddress; the re-flag re-snapshots totalBonus == 0.
function test_bug_sweepBeforeReflag_understatesBounty() external {
_stake(alice, 100 * ONE); // principal S = 100
_contributeBonus(carol, 40 * ONE); // bonus B = 40
assertEq(pool.riskWindowStart(), 0, "no active-risk observed");
​
// Registry is CORRUPTED but (moderator judges) the breach was OUT of this pool's scope,
// so the moderator first flags SURVIVED. Valid per flagOutcome (CORRUPTED registry allowed
// for SURVIVED). riskWindowStart stays 0.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(pool.riskWindowStart(), 0, "still no risk window");
​
// Permissionless: anyone sweeps. riskWindowStart==0 => reserve = principal only, so the
// FULL bonus (40) is swept to recoveryAddress and totalBonus is zeroed. Does NOT set
// claimsStarted, so the moderator's re-flag window stays open.
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, 40 * ONE, "full bonus swept to recovery");
assertEq(pool.totalBonus(), 0, "totalBonus zeroed");
assertEq(pool.claimsStarted(), false, "re-flag window still open");
​
// Moderator realises the breach was in-scope and corrects to good-faith CORRUPTED.
// Supported correction (claimsStarted == false). But snapshotTotalBonus now re-reads 0.
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
// BUG: bounty is understated to the principal only — the whitehat's entitled bonus is gone.
assertEq(pool.bountyEntitlement(), 100 * ONE, "BUG: bounty = stake only, bonus lost");
​
vm.prank(attacker);
pool.claimAttackerBounty();
​
// Whitehat receives 100 instead of the intended 140. The 40 bonus sits at recoveryAddress.
assertEq(token.balanceOf(attacker), 100 * ONE, "BUG: whitehat shorted the 40 bonus");
assertEq(token.balanceOf(recovery) - recoveryBefore, 40 * ONE, "40 bonus misallocated to sponsor recovery");
​
emit log_named_uint("whitehat_received", token.balanceOf(attacker) / ONE);
emit log_named_uint("whitehat_entitled_control", 140);
emit log_named_uint("bonus_lost_to_recovery", (token.balanceOf(recovery) - recoveryBefore) / ONE);
}
}

Executed result:

[PASS] test_control_whitehatGetsFullPool() (gas: 571964) -> whitehat receives 140 (stake + bonus)
[PASS] test_bug_sweepBeforeReflag_understatesBounty() (gas: 599289)
whitehat_received: 100
whitehat_entitled_control: 140
bonus_lost_to_recovery: 40
Suite result: ok. 2 passed; 0 failed; 0 skipped

Recommended Mitigation

Option A (preferred, minimal) — lock the re-flag window once a material bonus has been swept, so a CORRUPTED correction after a real bonus sweep reverts instead of understating the bounty. The §4 dust-donation exemption is preserved (dust sweeps still don't lock the window):

// in sweepUnclaimedBonus, the riskWindowStart == 0 branch after the bonus has been swept out:
if (totalEligibleStake == 0 || riskWindowStart == 0) {
uint256 swept = amount <= totalBonus ? amount : totalBonus;
totalBonus -= swept;
+ // A material bonus has left the pool; the outcome must no longer be re-flaggable, otherwise a
+ // CORRUPTED re-flag re-snapshots totalBonus == 0 and understates the whitehat's bounty.
+ // Keep the §4 dust exemption: only lock when the swept amount is non-dust.
+ if (swept > DUST_THRESHOLD) claimsStarted = true;
}

Option B — don't release the bonus while the outcome is still re-flaggable. Reserve the bonus even when riskWindowStart == 0, and only pay recoveryAddress once claimsStarted == true, so a later CORRUPTED correction can still pay the whitehat the whole pool. (Larger change; Option A is the smaller, doc-consistent fix.)

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!