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

Part of a good-faith attacker's bounty entitlement can be silently drained to `recoveryAddress` before a corrective re-flag lands

Author Revealed upon completion

Root + Impact

Description

  • flagOutcome() lets the moderator correct a prior outcome call as long as claimsStarted is still false. This window exists specifically so a moderator can fix a wrong scope judgement (e.g. an initial SURVIVED later found to actually be an in-scope CORRUPTED) before any participant's payout locks in. The codebase's own design doc states the safety argument explicitly: a corrective re-flag is safe because "once value has left the contract, a corrective re-flag cannot be honored without breaking balance accounting" i.e. the guarantee only holds if no value leaves before claimsStarted flips.

  • sweepUnclaimedBonus() breaks that guarantee. It transfers real funds to recoveryAddress whenever riskWindowStart == 0, but deliberately never sets claimsStarted. This means the "safe" re-flag window can remain open after real value has already left the contract exactly the condition the design doc assumes can't happen. When a moderator later corrects SURVIVED to a good-faith CORRUPTED, bountyEntitlement is re-snapshotted from totalBonus, which is now missing whatever was already swept so the named whitehat's entitlement silently excludes the bonus, which sits with recoveryAddress instead.

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();
// ... SURVIVED / CORRUPTED branch validation omitted ...
​
bool willBeGoodFaithCorrupted = newOutcome == PoolStates.Outcome.CORRUPTED && goodFaith_;
​
outcome = newOutcome;
goodFaith = goodFaith_;
attacker = attacker_;
snapshotTotalStaked = totalEligibleStake;
@> snapshotTotalBonus = totalBonus; // <-- re-reads totalBonus, unaware it may already be drained
snapshotSumStakeTime = sumStakeTime;
snapshotSumStakeTimeSq = sumStakeTimeSq;
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;
@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;
// ... deadline bookkeeping omitted ...
}
​
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.
@> stakeToken.safeTransfer(recoveryAddress, amount);
​
emit BonusSwept(msg.sender, recoveryAddress, amount);
}

Risk

Likelihood:

  • Occurs whenever a moderator's first-pass outcome call needs correcting an explicitly designed-for scenario (the re-flag window exists specifically for this), combined with riskWindowStart == 0, which happens routinely for a compromise that isn't observed by the pool during its brief UNDER_ATTACK window before flipping to CORRUPTED.

  • sweepUnclaimedBonus() is fully permissionless and repeat-callable, and the sponsor has a direct economic incentive to call it the instant any SURVIVED lands with riskWindowStart == 0 it costs nothing and functions as a race against a possible correction, not an edge case someone has to construct carefully.

Impact:

  • The good-faith whitehat's bounty entitlement, documented as "the whole pool: stake + bonus," silently drops to stake-only, with no revert or signal anywhere in the flow claimAttackerBounty() succeeds normally and simply pays out less.

  • recoveryAddress (sponsor-controlled) permanently captures value that the corrected, final, on-chain-recorded outcome designates for the attacker, a direct, deterministic fund misdirection once the sequence occurs, not a probabilistic or griefing-only outcome.

Proof of Concept

// 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";
import {MockERC20} from "test/mocks/MockERC20.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
​
contract ReflagBonusDivergencePoCTest is Test {
uint256 internal constant ONE = 1e18;
​
MockERC20 internal token;
MockAttackRegistry internal attackRegistry;
MockSafeHarborRegistry internal safeHarborRegistry;
MockAgreement internal agreement;
ConfidencePool internal pool;
​
address internal sponsor = makeAddr("sponsor");
address internal moderator = makeAddr("moderator");
address internal recovery = makeAddr("recovery");
address internal staker = makeAddr("staker");
address internal donor = makeAddr("donor");
address internal whitehat = makeAddr("whitehat"); // the REAL good-faith attacker
address internal scopeAccount = address(0xC0FFEE);
​
function setUp() public {
token = new MockERC20();
attackRegistry = new MockAttackRegistry();
safeHarborRegistry = new MockSafeHarborRegistry();
agreement = new MockAgreement(sponsor);
agreement.setContractInScope(scopeAccount, true);
​
safeHarborRegistry.setAttackRegistry(address(attackRegistry));
safeHarborRegistry.setAgreementValid(address(agreement), true);
​
ConfidencePool implementation = new ConfidencePool();
pool = ConfidencePool(Clones.clone(address(implementation)));
​
address[] memory scope = new address[](1);
scope[0] = scopeAccount;
​
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
​
pool.initialize(
address(agreement), address(token), address(safeHarborRegistry), moderator,
block.timestamp + 60 days, ONE, recovery, sponsor, scope
);
}
​
function test_bonusDivertedFromGoodFaithAttacker_viaReflagWindow() public {
token.mint(staker, 100 ether);
vm.startPrank(staker);
token.approve(address(pool), 100 ether);
pool.stake(100 ether);
vm.stopPrank();
​
token.mint(donor, 20 ether);
vm.startPrank(donor);
token.approve(address(pool), 20 ether);
pool.contributeBonus(20 ether);
vm.stopPrank();
​
// registry jumps straight to CORRUPTED; riskWindowStart never seals
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
assertEq(pool.riskWindowStart(), 0);
​
// moderator's first-pass call: looks out-of-scope
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertFalse(pool.claimsStarted());
​
// anyone sweeps the "unowed" bonus immediately
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, 20 ether);
assertFalse(pool.claimsStarted()); // still open for re-flag
​
// moderator corrects: this WAS in-scope, good-faith CORRUPTED, real whitehat
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
​
// entitlement silently excludes the already-swept bonus
assertEq(pool.bountyEntitlement(), 100 ether); // docs say it should be 120 ether
​
vm.prank(whitehat);
pool.claimAttackerBounty();
​
assertEq(token.balanceOf(whitehat), 100 ether); // shorted by exactly the swept bonus
assertEq(token.balanceOf(recovery) - recoveryBefore, 20 ether); // sponsor keeps it
}
}

Run: forge test --match-contract ReflagBonusDivergencePoCTest -vvv — passes clean.

Recommended Mitigation

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);
+ // A CORRUPTED-eligible registry state means the moderator could still legitimately
+ // re-flag SURVIVED -> good-faith CORRUPTED, in which case sweeping now would silently
+ // remove funds the eventual whitehat is entitled to. Close the re-flag window here too,
+ // the same way every other value-moving path does.
+ if (!claimsStarted) claimsStarted = true;
+ stakeToken.safeTransfer(recoveryAddress, amount);
​
emit BonusSwept(msg.sender, recoveryAddress, amount);

Note this reopens the narrower griefing concern the original comment was solving (a 1-wei donation could let anyone force claimsStarted = true and lock the re-flag window). A more complete fix pairs this change with a minimum-amount or minimum-delay gate on the first sweepUnclaimedBonus() call, so dust can't be used to force early finality while genuine sweeps still close the window as intended.

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!