When createPool() clones a new pool and calls initialize(), the initialization function uses OpenZeppelin's initializer modifier. If initialize() reverts for any reason, the initialized flag remains true, permanently bricking the clone. The salt used for deterministic deployment is also burned, preventing any future pool from using the same agreement index.
The initializer modifier in OpenZeppelin sets the initialized flag before executing the function body. If the function later reverts, the flag stays true, making it impossible to re-initialize the contract.
Root cause
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();
}
_replaceScope(accounts);
}
function createPool(...) external whenNotPaused returns (address pool) {
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
pool = Clones.cloneDeterministic(poolImplementation, salt);
IConfidencePool(pool).initialize(...);
_poolsByAgreement[agreement].push(pool);
}
Risk
Likelihood:
A malicious agreement owner could create an agreement with invalid scope validation, then create a pool that reverts during _replaceScope(), burning the salt
A malicious registry could report invalid agreements at specific times to cause initialization failures
Validators that change state between the factory's isAgreementValid() check and the pool's internal validation could cause failures
Impact:
The deployed clone is permanently unusable (bricked)
The deterministic salt is burned; no other pool can use that agreement index
Gas spent by the caller is wasted
Could be used as a griefing vector against specific agreements
Proof of Concept
function testFailedInitBricksClone() public {
address maliciousAgreement = deployMaliciousAgreement();
vm.startPrank(agreementOwner);
vm.expectRevert(abi.encodeWithSelector(AccountNotInAgreementScope.selector));
factory.createPool(
maliciousAgreement,
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts
);
vm.expectRevert();
factory.createPool(
maliciousAgreement,
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
validAccounts
);
}
Recommended Mitigation
- remove this codev
+ add this // ConfidencePoolFactory.sol
function createPool(
address agreement,
address stakeToken,
uint256 expiry,
uint256 minStake,
address recoveryAddress,
address[] calldata accounts
) external whenNotPaused returns (address pool) {
if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
+ // Pre-validate all scope accounts before cloning
+ for (uint256 i = 0; i < accounts.length; i++) {
+ if (!IAgreement(agreement).isContractInScope(accounts[i])) {
+ revert AccountNotInAgreementScope(accounts[i]);
+ }
+ }
+
+ // Validate no duplicates
+ for (uint256 i = 0; i < accounts.length; i++) {
+ for (uint256 j = i + 1; j < accounts.length; j++) {
+ if (accounts[i] == accounts[j]) revert DuplicateAccount(accounts[i]);
+ }
+ }
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
pool = Clones.cloneDeterministic(poolImplementation, salt);
IConfidencePool(pool).initialize(
agreement,
stakeToken,
address(safeHarborRegistry),
defaultOutcomeModerator,
expiry,
minStake,
recoveryAddress,
msg.sender,
accounts
);
_poolsByAgreement[agreement].push(pool);
emit PoolCreated(
agreement,
pool,
stakeToken,
expiry,
minStake,
recoveryAddress,
defaultOutcomeModerator,
address(safeHarborRegistry)
);
}