BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: medium

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.

Support

FAQs

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

Give us feedback!