BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: low

`createPool` allows creation of a non-functional pool with an empty scope

Author Revealed upon completion

Root + Impact

Description

The ConfidencePoolFactory.createPool function allows a sponsor to deploy a new ConfidencePool. It accepts an accounts array for the initial scope but does not validate that this array is non-empty. The ConfidencePool.initialize function, which is called internally, does revert if the accounts array is empty.

Because the check occurs in the clone's initializer rather than the factory, a sponsor can call createPool with an empty accounts array. This transaction will successfully deploy a clone to a deterministic address, consume gas, and then revert during the initialization step. This leaves a deployed but uninitialized and unusable contract on-chain, and prevents a new, valid pool from being created for the same agreement with the same index, as the deterministic address is now occupied.

// Root cause in the codebase with @> marks to highlight the relevant section
// In ConfidencePoolFactory.sol
function createPool(
// ...
address[] calldata accounts
) external whenNotPaused returns (address pool) {
@> // No check here to prevent accounts.length == 0
// ...
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
pool = Clones.cloneDeterministic(poolImplementation, salt);
​
@> IConfidencePool(pool)
.initialize(
// ...
@> accounts // This call will revert if accounts is empty
);
// ...
}
​
// In ConfidencePool.sol
function _replaceScope(address[] calldata accounts) internal {
@> if (accounts.length == 0) revert EmptyScope();
// ...
}
​

Risk

Likelihood: Low

  • Reason 1

    • This occurs when a pool sponsor makes a mistake and calls createPool with an empty accounts parameter.

Impact: Mid

  • Impact 1

    • A non-functional, uninitialized ConfidencePool clone is deployed, polluting the blockchain.

  • Impact 2

    • The sponsor loses the gas fees for the failed transaction.

  • Impact 3

    • It prevents a functional pool from being created at that deterministic address, effectively "burning" the pool index for that agreement and forcing the sponsor to use a different configuration if they wish to try again.

Proof of Concept

// Add this test to a relevant test file, like `ConfidencePoolFactory.t.sol`
​
function test_PoC_createPool_withEmptyScope_leavesUninitializedClone() public {
// 1. Setup: Create an agreement and allow the stake token.
address agreementOwner = makeAddr("agreementOwner");
vm.prank(BATTLECHAIN_REGISTRY_OWNER);
address agreement = safeHarborRegistry.createAgreement(agreementOwner, "test agreement");
​
vm.prank(factoryOwner);
factory.setStakeTokenAllowed(address(stakeToken), true);
​
// 2. Calculate the deterministic address for the new pool.
uint256 poolIndex = factory.poolCountByAgreement(agreement); // will be 0
bytes32 salt = keccak256(abi.encode(agreement, poolIndex));
address predictedAddress = Clones.predictDeterministicAddress(address(poolImplementation), salt, address(factory));
​
// Pre-condition: The predicted address should be empty.
assertEq(predictedAddress.code.length, 0, "Predicted address should be empty before creation");
​
// 3. Attack: The sponsor calls createPool with an empty scope array.
// The call to the factory is expected to revert because the internal `initialize`
// call on the new pool clone will revert.
address[] memory emptyScope;
vm.prank(agreementOwner);
vm.expectRevert(IConfidencePool.EmptyScope.selector);
factory.createPool(
agreement,
address(stakeToken),
block.timestamp + 31 days,
1e18,
makeAddr("recovery"),
emptyScope
);
​
// 4. Verification: Although the transaction reverted, a contract was deployed.
// The clone is created *before* the reverting initialize call, leaving a
// non-functional, uninitialized contract at the predicted address.
assertTrue(predictedAddress.code.length > 0, "A contract should have been deployed at the predicted address");
​
// We can also check that the pool count was NOT incremented, as the push happens after the revert.
assertEq(factory.poolCountByAgreement(agreement), 0, "Pool count should not have incremented");
​
log("PoC successful: createPool with empty scope deployed a useless, uninitialized contract.");
}
​

Recommended Mitigation

Add a check in ConfidencePoolFactory.createPool to ensure the accounts array is not empty before attempting to create the clone. This provides a faster, cheaper failure and prevents the creation of a useless contract.

if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
+ if (accounts.length == 0) revert IConfidencePool.EmptyScope();
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
// aderyn-fp-next-line(reentrancy-state-change)
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
​
​

Support

FAQs

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

Give us feedback!