BattleChain Confidence Pools

Cyfrin

FoundrySolidityLayer 2

7.25 ETH

View all submissions

Previous Next

Submission Details

Severity: low

Valid

sweepUnclaimedBonus permissionlessly drains bonus funds before the moderator's pre-claim correction window closes, permanently shorting a later good-faith CORRUPTED attacker bounty

Author Revealed upon completion

Description

Normal behavior: flagOutcome gives the moderator a pre-claim correction window — the outcome can be re-flagged any time before the first claim (claimsStarted == false), so a mistaken initial flag can be fixed before funds move to the wrong party.

The issue: sweepUnclaimedBonus() is permissionless and callable the moment outcome is SURVIVED, and it deliberately never sets claimsStarted. When riskWindowStart == 0 — reachable via AttackRegistry.goToProduction(), a normal, unrestricted path that skips the attack phase entirely — the function treats the whole bonus as unowed and sweeps 100% of it to recoveryAddress immediately. If the moderator then corrects the outcome to good-faith CORRUPTED and names a whitehat attacker, bountyEntitlement is recomputed from snapshotTotalBonus, which is now 0 because the bonus already left the contract. The attacker's bounty is silently short-changed by the swept amount with no revert and no recovery path.

function sweepUnclaimedBonus() external nonReentrant {

if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {

revert OutcomeNotEligibleForSweep();

}

@> // Bonus is only "reserved" (protected) when a risk window was observed.

@> // riskWindowStart == 0 is a normal, reachable state (see goToProduction), not an edge case.

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 -- this is exactly what leaves the

@> // moderator's pre-claim correction window open for this sweep to slip through.

stakeToken.safeTransfer(recoveryAddress, amount);

emit BonusSwept(msg.sender, recoveryAddress, amount);

}

function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {

@> // Correction window: re-flag allowed while claimsStarted is false.

@> // sweepUnclaimedBonus runs inside this exact window without tripping this guard.

if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();

// ...

bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;

Risk

Likelihood:

The moderator flags SURVIVED while riskWindowStart == 0, which occurs whenever the sponsor calls the permissionless, precondition-free AttackRegistry.goToProduction() — a documented normal usage path, not a rare edge case.
Any address calls sweepUnclaimedBonus() in the same window before the moderator corrects a wrong flag, since nothing gates timing between flagOutcome and sweepUnclaimedBonus, and the call requires no special permission or coordination.

Impact:

A legitimate whitehat attacker's bounty is silently reduced by the full bonus amount, with the shortfall permanently redirected to recoveryAddress instead of the party who found the real corruption.
No revert or event signals the shortfall — the loss is only detectable by comparing expected vs. actual payout after the fact, with no on-chain path to recover the difference.

Proof of Concept

Save the following as a single file, e.g. test/poc/SweepBeforeCorrection.t.sol — it is fully self-contained (mocks included in the same file, no other test files required) and only depends on the repo's own src/ contracts and its existing lib/ dependencies (OpenZeppelin + the battlechain-safe-harbor-contracts submodule).

// SPDX-License-Identifier: MIT

pragma solidity 0.8.26;

import "forge-std/Test.sol";

import "src/ConfidencePool.sol";

import "src/libraries/PoolStates.sol";

import {IBattleChainSafeHarborRegistry} from "@battlechain/interface/IBattleChainSafeHarborRegistry.sol";

import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";

import {IAgreement} from "@battlechain/interface/IAgreement.sol";

import {Account, Chain, BountyTerms, AgreementDetails, Contact} from "@battlechain/types/AgreementTypes.sol";

import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";

import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

// ---------------------------------------------------------------------------

// Self-contained mocks (inlined so this PoC has zero external file dependencies)

// ---------------------------------------------------------------------------

contract MockAttackRegistry is IAttackRegistry {

ContractState public state = ContractState.NOT_DEPLOYED;

function setState(ContractState s) external { state = s; }

function getAgreementState(address) external view returns (ContractState) { return state; }

function approveAttack(address) external pure { revert("stub"); }

function authorizeAgreementOwner(address, address) external pure { revert("stub"); }

function cancelPromotion(address) external pure { revert("stub"); }

function changeRegistryModerator(address) external pure { revert("stub"); }

function getAgreementForContract(address) external pure returns (address) { revert("stub"); }

function getAgreementInfo(address) external pure returns (AgreementInfo memory) { revert("stub"); }

function getAttackModerator(address) external pure returns (address) { revert("stub"); }

function getAuthorizedOwner(address) external pure returns (address) { revert("stub"); }

function getRegistryModerator() external pure returns (address) { revert("stub"); }

function getSafeHarborRegistry() external pure returns (address) { revert("stub"); }

function goToProduction(address) external pure { revert("stub"); }

function instantCorrupt(address) external pure { revert("stub"); }

function instantPromote(address) external pure { revert("stub"); }

function isTopLevelContractUnderAttack(address) external pure returns (bool) { revert("stub"); }

function markCorrupted(address) external pure { revert("stub"); }

function promote(address) external pure { revert("stub"); }

function registerContractForExistingAgreement(address) external pure { revert("stub"); }

function registerDeployment(address, address) external pure { revert("stub"); }

function rejectAttackRequest(address, bool) external pure { revert("stub"); }

function requestUnderAttack(address) external pure { revert("stub"); }

function requestUnderAttackByNonAuthorized(address) external pure { revert("stub"); }

function requestUnderAttackForUnverifiedContracts(address) external pure { revert("stub"); }

function setSafeHarborRegistry(address) external pure { revert("stub"); }

function syncNewContracts(address) external pure { revert("stub"); }

function transferAttackModerator(address, address) external pure { revert("stub"); }

function unregisterContractForExistingAgreement(address) external pure { revert("stub"); }

}

contract MockAgreement is IAgreement {

address public owner_;

mapping(address => bool) public inScope;

constructor(address o) { owner_ = o; }

function owner() external view returns (address) { return owner_; }

function setInScope(address a, bool v) external { inScope[a] = v; }

function isContractInScope(address a) external view returns (bool) { return inScope[a]; }

function addAccounts(string memory, Account[] memory) external pure { revert("stub"); }

function addOrSetChains(Chain[] memory) external pure { revert("stub"); }

function extendCommitmentWindow(uint256) external pure { revert("stub"); }

function getAgreementURI() external pure returns (string memory) { revert("stub"); }

function getBattleChainCaip2ChainId() external pure returns (string memory) { revert("stub"); }

function getBattleChainScopeAddresses() external pure returns (address[] memory) { revert("stub"); }

function getBattleChainScopeCount() external pure returns (uint256) { revert("stub"); }

function getBountyTerms() external pure returns (BountyTerms memory) { revert("stub"); }

function getCantChangeUntil() external pure returns (uint256) { revert("stub"); }

function getChainIds() external pure returns (string[] memory) { revert("stub"); }

function getDetails() external pure returns (AgreementDetails memory) { revert("stub"); }

function getProtocolName() external pure returns (string memory) { revert("stub"); }

function getRegistry() external pure returns (address) { revert("stub"); }

function removeAccounts(string memory, string[] memory) external pure { revert("stub"); }

function removeChains(string[] memory) external pure { revert("stub"); }

function setAgreementURI(string memory) external pure { revert("stub"); }

function setBountyTerms(BountyTerms memory) external pure { revert("stub"); }

function setContactDetails(Contact[] memory) external pure { revert("stub"); }

function setProtocolName(string memory) external pure { revert("stub"); }

}

contract MockRegistry is IBattleChainSafeHarborRegistry {

address public attackRegistry_;

mapping(address => bool) public valid;

constructor(address ar) { attackRegistry_ = ar; }

function setValid(address a, bool v) external { valid[a] = v; }

function isAgreementValid(address a) external view returns (bool) { return valid[a]; }

function getAttackRegistry() external view returns (address) { return attackRegistry_; }

function adoptSafeHarbor(address) external pure { revert("stub"); }

function getAgreement(address) external pure returns (address) { revert("stub"); }

function getAgreementFactory() external pure returns (address) { revert("stub"); }

function isChainValid(string calldata) external pure returns (bool) { revert("stub"); }

function setAgreementFactory(address) external pure { revert("stub"); }

}

contract MockStakeToken is ERC20 {

constructor() ERC20("Mock", "MCK") { _mint(msg.sender, 1_000_000e18); }

}

// ---------------------------------------------------------------------------

// PoC

// ---------------------------------------------------------------------------

contract SweepBeforeCorrectionTest is Test {

ConfidencePool pool;

MockRegistry registry;

MockAttackRegistry attackRegistry;

MockAgreement agreement;

MockStakeToken token;

address sponsor = address(0xA11CE);

address moderator = address(0xB0B);

address staker = address(0xCAFE);

address recovery = address(0xD00D);

address attacker = address(0xEEEE);

function setUp() public {

attackRegistry = new MockAttackRegistry();

registry = new MockRegistry(address(attackRegistry));

agreement = new MockAgreement(sponsor);

token = new MockStakeToken();

registry.setValid(address(agreement), true);

agreement.setInScope(address(0x1), true);

// ConfidencePool's constructor calls _disableInitializers(), so the implementation

// itself can never be initialized -- deploy it, then clone it (as the factory does),

// and initialize the clone.

ConfidencePool implementation = new ConfidencePool();

pool = ConfidencePool(Clones.clone(address(implementation)));

address[] memory accounts = new address[](1);

accounts[0] = address(0x1);

vm.prank(sponsor);

pool.initialize(

address(agreement),

address(token),

address(registry),

moderator,

block.timestamp + 31 days,

1e18,

recovery,

sponsor,

accounts

);

token.transfer(staker, 100e18);

vm.startPrank(staker);

token.approve(address(pool), 100e18);

pool.stake(100e18); // riskWindowStart stays 0 (never observed active-risk state)

vm.stopPrank();

token.transfer(sponsor, 10e18);

vm.startPrank(sponsor);

token.approve(address(pool), 10e18);

pool.contributeBonus(10e18);

vm.stopPrank();

}

function test_bonusSweptBeforeCorrectionShortsAttacker() public {

// Agreement reaches PRODUCTION via a real, permissionless path

// (AttackRegistry.goToProduction skips the attack phase entirely, no precondition).

attackRegistry.setState(IAttackRegistry.ContractState.PRODUCTION);

vm.prank(moderator);

pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));

// Anyone sweeps the "unclaimed" bonus immediately -- inside the correction window.

uint256 recoveryBefore = token.balanceOf(recovery);

pool.sweepUnclaimedBonus();

uint256 sweptAmount = token.balanceOf(recovery) - recoveryBefore;

assertEq(sweptAmount, 10e18, "full bonus swept before correction");

// Moderator corrects: agreement was actually breached, names the whitehat.

attackRegistry.setState(IAttackRegistry.ContractState.CORRUPTED);

vm.prank(moderator);

pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);

vm.prank(attacker);

pool.claimAttackerBounty();

// Attacker receives only the 100e18 stake -- the 10e18 bonus never reaches them.

assertEq(token.balanceOf(attacker), 100e18, "attacker under-paid by the pre-swept bonus");

}

How to run:

forge test --match-path test/poc/SweepBeforeCorrection.t.sol -vvvv

Result: passes. Trace confirms 10e18 swept to recovery before correction, and the attacker receives exactly 100e18 (principal only) instead of the full 110e18 (principal + bonus) entitlement.

Note on inlined struct import path: Account, Chain, BountyTerms, AgreementDetails, and Contact are declared in lib/battlechain-safe-harbor-contracts/src/types/AgreementTypes.sol. If your project's remapping for @battlechain/ differs, adjust that single import line to match your remappings.txt.

Recommended Mitigation

function sweepUnclaimedBonus() external nonReentrant {

if (outcome != PoolStates.Outcome.SURVIVED && outcome != PoolStates.Outcome.EXPIRED) {

revert OutcomeNotEligibleForSweep();

}

+ // Block the sweep while the moderator's pre-claim correction window is still open,

+ // so a mis-flagged SURVIVED can't have its bonus permanently drained before being fixed.

+ if (!claimsStarted) revert CorrectionWindowStillOpen();

uint256 reserved;

if (totalEligibleStake != 0) {

reserved = totalEligibleStake;

if (riskWindowStart != 0) {

reserved += snapshotTotalBonus - claimedBonus;

}

This closes the gap without reintroducing the griefing vector the original design avoided: claimsStarted is only ever set by genuine value-movement claims (claimSurvived, claimExpired, claimCorrupted, claimAttackerBounty), so a 1-wei donation still cannot trigger it. The tradeoff is that sweepUnclaimedBonus now requires at least one legitimate claim to have already occurred — which matches the intended sequence anyway, since bonus sweeping is meant to run after stakers have had the chance to claim against a finalized outcome, not before the correction window has even closed.

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.

Prize pool breakdown

Total prize

7.25 ETH

nSLOC

589

0 ETH / LOC

High Medium

Low

1.25

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

View preliminary results

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

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

Give us feedback!