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

`sweepUnclaimedBonus` permanently diverts the whitehat's bonus entitlement to `recoveryAddress`

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: when a pool resolves SURVIVED/EXPIRED with riskWindowStart == 0 ("no observed risk"; the registry skipped the active-risk states entirely), _bonusShare owes the bonus to no staker, so sweepUnclaimedBonus() is permissionlessly allowed to recover the unowed bonus to recoveryAddress. Separately, because flagOutcome only closes its re-flag window once claimsStarted is true, and sweepUnclaimedBonus() deliberately does not set claimsStarted, a moderator can still re-flag the pool as good-faith CORRUPTED afterward if the agreement is genuinely breached later, naming the real whitehat attacker and re-snapshotting their bountyEntitlement off the live totalBonus/totalEligibleStake.

  • The problem: sweepUnclaimedBonus() physically transfers the bonus out and zeroes totalBonus when it decides the bonus is currently unowed, but it does this irreversibly, even though it intentionally keeps the re-flag window open for exactly the scenario where that bonus becomes owed to someone else moments later. When the moderator subsequently re-flags good-faith CORRUPTED, flagOutcome re-snapshots snapshotTotalBonus = totalBonus, which is now 0. The named whitehat's bountyEntitlement silently excludes the bonus, which is unrecoverably sitting at recoveryAddress instead of following the attacker's bounty as the CORRUPTED/good-faith payout rule intends.

function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
uint256 reserved;
if (totalEligibleStake != 0) {
reserved = totalEligibleStake;
if (riskWindowStart != 0) {
reserved += snapshotTotalBonus - claimedBonus;
}
}
uint256 freeBalance = stakeToken.balanceOf(address(this));
uint256 amount = freeBalance > reserved ? freeBalance - reserved : 0;
if (amount == 0) revert NothingToSweep();
if (totalEligibleStake == 0 || riskWindowStart == 0) {
//> totalBonus -= amount <= totalBonus ? amount : totalBonus; // @> bonus zeroed out permanently
}
// @> Intentionally does NOT set claimsStarted -- re-flag window (flagOutcome) stays open
//> stakeToken.safeTransfer(recoveryAddress, amount); // @> bonus irreversibly leaves the pool
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
// @> re-flag still allowed here because sweepUnclaimedBonus never set claimsStarted
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
...
outcome = newOutcome;
goodFaith = goodFaith_;
attacker = attacker_;
snapshotTotalStaked = totalEligibleStake;
//> snapshotTotalBonus = totalBonus; // @> re-snapshots off the ALREADY-SWEPT (zeroed) totalBonus
...
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;
// @> bountyEntitlement excludes the bonus that already left for recoveryAddress
}

Risk

Likelihood:

  • Occurs whenever a pool resolves SURVIVED/EXPIRED with riskWindowStart == 0 (registry transitioned straight from pre-attack staging to a terminal state, e.g. NEW_DEPLOYMENT PRODUCTION) a state the protocol's own docs treat as a normal, expected edge case, not a rare fault.

  • sweepUnclaimedBonus() is permissionless and requires no staker action first, so any observer (a bot, a competing sponsor, or recoveryAddress's own owner) will sweep the bonus as soon as it becomes sweepable, well before the pool's underlying agreement's real fate is known.

  • The re-flag path is a documented, intended moderator workflow (correcting an outcome, naming a whitehat after the fact when a real attack surfaces post-"resolution"); it is not a contrived edge case; it is the exact mechanism sweepUnclaimedBonus was written to stay compatible with.

Impact:

  • The bonus (the sponsor-funded incentive specifically meant to reward whoever ends up entitled to it under a CORRUPTED outcome) settles at recoveryAddress controlled by the sponsor/pool owner instead of the whitehat attacker the moderator names in good faith.

  • Breaks the documented CORRUPTED/good-faith payout guarantee that the full pool (stake + bonus) is owed to the named attacker, silently reducing it to stake-only with no on-chain signal that anything went wrong (the transaction succeeds, bountyEntitlement is simply smaller than expected).

Proof of Concept

Add this file to the mock folder

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IBattleChainSafeHarborRegistry} from "@battlechain/interface/IBattleChainSafeHarborRegistry.sol";
import {IAgreement} from "@battlechain/interface/IAgreement.sol";
​
/// @notice Minimal mock of the on-chain attack registry. State is set directly by the test so we
/// can drive the exact registry-state sequence the PoC needs. Does NOT inherit IAttackRegistry —
/// matching selectors is sufficient for an address-cast call site, and inheriting the full
/// interface would force struct-typed stubs that clash under via_ir.
contract MockAttackRegistry {
mapping(address => IAttackRegistry.ContractState) public state;
​
function setState(address agreement, IAttackRegistry.ContractState s) external {
state[agreement] = s;
}
​
function getAgreementState(address agreement) external view returns (IAttackRegistry.ContractState) {
return state[agreement];
}
}
​
/// @notice Minimal mock of the Safe Harbor registry singleton. Does NOT inherit
/// IBattleChainSafeHarborRegistry — matching selectors is sufficient for an address-cast call site.
contract MockSafeHarborRegistry {
address public attackRegistry;
mapping(address => bool) public validAgreements;
​
constructor(address attackRegistry_) {
attackRegistry = attackRegistry_;
}
​
function setAgreementValid(address agreement, bool valid_) external {
validAgreements[agreement] = valid_;
}
​
function getAttackRegistry() external view returns (address) {
return attackRegistry;
}
​
function isAgreementValid(address agreement) external view returns (bool) {
return validAgreements[agreement];
}
}
​
/// @notice Minimal mock of the underlying Safe Harbor agreement. Every address is in-scope so the
/// PoC doesn't need to fuss with scope bookkeeping. Does NOT inherit IAgreement — matching
/// selectors is sufficient for an address-cast call site.
contract MockAgreement {
function isContractInScope(address) external pure returns (bool) {
return true;
}
}
​
/// @notice Plain mintable ERC20 used as the pool's stake/bonus token.
contract MockStakeToken is ERC20 {
constructor() ERC20("Mock Stake Token", "MST") {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
​
​

Add this POC to the unit test folder.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test, console2} from "forge-std/Test.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
​
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
​
import {MockAttackRegistry, MockSafeHarborRegistry, MockAgreement, MockStakeToken} from "../mocks/PoCMocks.sol";
​
/// @title PoC: sweepUnclaimedBonus permanently diverts the whitehat's bounty
/// @notice Demonstrates the finding:
///
/// 1. The registry skips the active-risk states entirely (NEW_DEPLOYMENT -> PRODUCTION), so
/// `riskWindowStart` is never sealed ("no observed risk").
/// 2. `claimExpired()` auto-resolves the pool to SURVIVED off the observed PRODUCTION state.
/// 3. Because `riskWindowStart == 0`, nobody is owed any bonus share, so anyone can call
/// `sweepUnclaimedBonus()` to send the *entire* bonus pool to `recoveryAddress`. This
/// intentionally does NOT set `claimsStarted`, per the contract's own comment, so the
/// moderator's pre-claim re-flag window stays open.
/// 4. The agreement is later actually attacked and the registry moves to CORRUPTED. The
/// moderator (correctly, per the docs) re-flags the pool as good-faith CORRUPTED naming the
/// whitehat attacker.
/// 5. `flagOutcome` re-snapshots `snapshotTotalBonus = totalBonus`, but `totalBonus` was already
/// zeroed out (and the tokens physically moved to `recoveryAddress`) by step 3. The
/// attacker's `bountyEntitlement` therefore excludes the bonus entirely, and
/// `claimAttackerBounty()` only pays out stake, not stake + bonus. The bonus is
/// unrecoverably sitting at `recoveryAddress` instead.
contract BonusDivertedOnReflagTest is Test {
ConfidencePool internal pool;
MockAttackRegistry internal attackRegistry;
MockSafeHarborRegistry internal safeHarborRegistry;
MockAgreement internal agreement;
MockStakeToken internal token;
​
address internal owner = makeAddr("owner");
address internal moderator = makeAddr("moderator");
address internal recoveryAddress = makeAddr("recoveryAddress");
address internal staker = makeAddr("staker");
address internal whitehat = makeAddr("whitehat");
address internal randomCaller = makeAddr("randomCaller");
​
address internal scopedAccount = makeAddr("scopedAccount");
​
uint256 internal constant STAKE_AMOUNT = 100 ether;
uint256 internal constant BONUS_AMOUNT = 40 ether;
uint256 internal expiry;
​
function setUp() public {
attackRegistry = new MockAttackRegistry();
agreement = new MockAgreement();
safeHarborRegistry = new MockSafeHarborRegistry(address(attackRegistry));
safeHarborRegistry.setAgreementValid(address(agreement), true);
token = new MockStakeToken();
​
// Registry starts in pre-attack staging.
attackRegistry.setState(address(agreement), IAttackRegistry.ContractState.NEW_DEPLOYMENT);
​
ConfidencePool implementation = new ConfidencePool();
pool = ConfidencePool(Clones.clone(address(implementation)));
​
expiry = block.timestamp + 31 days;
​
address[] memory accounts = new address[](1);
accounts[0] = scopedAccount;
​
pool.initialize(
address(agreement),
address(token),
address(safeHarborRegistry),
moderator,
expiry,
1 ether, // minStake
recoveryAddress,
owner,
accounts
);
​
token.mint(staker, STAKE_AMOUNT);
token.mint(owner, BONUS_AMOUNT); // owner acts as bonus contributor for simplicity
​
vm.prank(staker);
token.approve(address(pool), STAKE_AMOUNT);
vm.prank(owner);
token.approve(address(pool), BONUS_AMOUNT);
​
vm.prank(staker);
pool.stake(STAKE_AMOUNT);
​
vm.prank(owner);
pool.contributeBonus(BONUS_AMOUNT);
}
​
function test_sweepUnclaimedBonus_divertsWhitehatBountyOnReflag() public {
// --- Step 1: registry skips active-risk states entirely and jumps straight to
// PRODUCTION. `riskWindowStart` never seals -- this is the documented "no observed risk"
// scenario.
attackRegistry.setState(address(agreement), IAttackRegistry.ContractState.PRODUCTION);
assertEq(pool.riskWindowStart(), 0, "riskWindowStart should never have sealed");
​
// --- Step 2: moderator flags SURVIVED off the observed PRODUCTION state via the
// moderator-driven `flagOutcome` path (not the permissionless `claimExpired`
// auto-resolution). `flagOutcome` does NOT set `claimsStarted` by itself, matching its
// own documented "re-flag allowed pre-claim" window.
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.SURVIVED));
assertEq(pool.snapshotTotalBonus(), BONUS_AMOUNT, "bonus snapshot should hold the full bonus");
assertFalse(pool.claimsStarted(), "no claim entrypoint has run yet");
assertEq(token.balanceOf(address(pool)), STAKE_AMOUNT + BONUS_AMOUNT);
​
// --- Step 3: because riskWindowStart == 0, nobody is owed any bonus share, so the whole
// bonus is "sweepable". Anyone can call this permissionlessly.
vm.prank(randomCaller);
pool.sweepUnclaimedBonus();
​
assertEq(token.balanceOf(recoveryAddress), BONUS_AMOUNT, "entire bonus already sent to recoveryAddress");
assertEq(pool.totalBonus(), 0, "totalBonus zeroed out by the sweep");
assertFalse(pool.claimsStarted(), "sweep intentionally leaves claimsStarted false");
assertEq(token.balanceOf(address(pool)), STAKE_AMOUNT, "only stake principal remains in the pool");
​
// --- Step 4: the agreement is genuinely attacked after the fact. The registry moves to
// CORRUPTED, and the moderator does exactly what the docs say a moderator should do: names
// the whitehat attacker in a good-faith CORRUPTED re-flag. This is allowed because
// claimsStarted is still false.
attackRegistry.setState(address(agreement), IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
​
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED));
assertTrue(pool.goodFaith());
assertEq(pool.attacker(), whitehat);
​
// --- Step 5: the re-snapshot at flagOutcome captured totalBonus AFTER the sweep, i.e.
// zero. The whitehat's entitlement excludes the bonus that DESIGN.md says should have
// gone to them (or at worst waited for the moderator, never been auto-swept to
// recoveryAddress while a real CORRUPTED outcome was still possible).
assertEq(
pool.bountyEntitlement(),
STAKE_AMOUNT,
"bountyEntitlement excludes the bonus - it was already swept away"
);
​
vm.prank(whitehat);
pool.claimAttackerBounty();
​
assertEq(token.balanceOf(whitehat), STAKE_AMOUNT, "whitehat only recovers stake, never the bonus");
assertEq(
token.balanceOf(recoveryAddress),
BONUS_AMOUNT,
"the full bonus stays stuck at recoveryAddress instead of the whitehat bounty"
);
​
console2.log("whitehat payout: ", token.balanceOf(whitehat));
console2.log("recoveryAddress payout: ", token.balanceOf(recoveryAddress));
console2.log("bonus that should have followed the CORRUPTED bounty:", BONUS_AMOUNT);
}
}

Recommended Mitigation

function sweepUnclaimedBonus() external nonReentrant {
if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {
revert OutcomeNotEligibleForSweep();
}
uint256 reserved;
if (totalEligibleStake != 0) {
reserved = totalEligibleStake;
if (riskWindowStart != 0) {
reserved += snapshotTotalBonus - claimedBonus;
}
}
uint256 freeBalance = stakeToken.balanceOf(address(this));
uint256 amount = freeBalance > reserved ? freeBalance - reserved : 0;
if (amount == 0) revert NothingToSweep();
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
- // Intentionally does NOT set claimsStarted. A direct-transfer donation of as little as 1
- // wei would otherwise let anyone flip the flag post-flagOutcome and block the moderator's
- // documented pre-claim re-flag window. Genuine reliance only comes from claim entrypoints.
+ // Moving real bonus funds out of the pool IS genuine reliance -- it must close the
+ // re-flag window the same way every other value-moving entrypoint does, otherwise a
+ // later good-faith CORRUPTED re-flag re-snapshots off an already-emptied totalBonus and
+ // silently strips the bonus from the whitehat's bountyEntitlement.
+ claimsStarted = true;
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}

This closes the re-flag window as soon as real value moves, matching every other claim entrypoint. If a 1-wei donation griefing a moderator's re-flag is a concern worth preserving against, prefer gating sweepUnclaimedBonus() behind a minimum post-resolution delay (mirroring MODERATOR_CORRUPTED_GRACE) long enough for a moderator to act on a freshly observed CORRUPTED state first, rather than leaving the sweep able to permanently strip funds out from under a still-open re-flag window.

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!