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

Failed Initialization Permanently Bricks Clones

Author Revealed upon completion

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

// ConfidencePool.sol
function initialize(
address agreement_,
address stakeToken_,
address safeHarborRegistry_,
address outcomeModerator_,
uint256 expiry_,
uint256 minStake_,
address recoveryAddress_,
address owner_,
address[] calldata accounts
) external initializer { // @> initializer flag set BEFORE function body executes
if (agreement_ == address(0)) revert ZeroAddress();
if (stakeToken_ == address(0)) revert ZeroAddress();
// ... more validations ...
// @> If ANY validation reverts, the clone remains deployed but permanently bricked
if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {
revert InvalidAgreement();
}
// ...
_replaceScope(accounts); // @> This can also revert (invalid accounts, duplicate accounts)
}
// ConfidencePoolFactory.sol
function createPool(...) external whenNotPaused returns (address pool) {
// ... validations ...
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
pool = Clones.cloneDeterministic(poolImplementation, salt); // @> Clone deployed
IConfidencePool(pool).initialize(...); // @> If this reverts, the clone is bricked
_poolsByAgreement[agreement].push(pool); // @> Never executed if initialize reverts
}

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

// Test: Failed initialization bricks the clone
function testFailedInitBricksClone() public {
// Setup: Agreement with invalid scope validation
address maliciousAgreement = deployMaliciousAgreement();
// isContractInScope always returns false
vm.startPrank(agreementOwner);
// First attempt: create pool with accounts that aren't in scope
// This will revert inside _replaceScope()
vm.expectRevert(abi.encodeWithSelector(AccountNotInAgreementScope.selector));
factory.createPool(
maliciousAgreement,
stakeToken,
block.timestamp + 30 days,
1 ether,
recoveryAddress,
accounts // Accounts not in scope
);
// The salt for index 0 is now burned
// Second attempt: same agreement, same index
// cloneDeterministic will revert because address already used
vm.expectRevert(); // Address collision
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)
);
}

Support

FAQs

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

Give us feedback!