Root + Impact

Description

• ConfidencePoolFactory.createPool is the sole entrypoint for creating pools; it validates the forwarded parameters and then deploys a deterministic clone whose initialize performs a second, stricter round of parameter validation.

• The factory only partially mirrors the clone's parameter checks: it enforces the expiry lower bound but omits the expiry <= type(uint32).max upper bound and the minStake != 0 check. Because the clone is deployed via Clones.cloneDeterministic before initialize runs, an out-of-range expiry or a zero minStake is only rejected after the CREATE2 deployment has executed, wasting that gas and surfacing a generic error from a different contract than the one the caller invoked.

Risk

Likelihood:

• Low. The offending inputs are caller-controlled and self-directed — only the agreement owner can call createPool (IAgreement(agreement).owner() != msg.sender gate), so this only fires on a sponsor mis-parameterizing their own pool creation, typically a fat-finger minStake = 0 or a UI passing an unbounded/huge expiry.

• Occurs on any createPool call carrying minStake == 0 or expiry > type(uint32).max; the transaction always reverts atomically, so it never produces a corrupt or half-deployed pool — the consequence is confined to the failed attempt.

Impact:

• Wasted gas: the full CREATE2 clone deployment runs and is then thrown away when initialize reverts within the same transaction.

• Degraded error surfacing: the caller receives a generic InvalidAmount / ExpiryTooFar originating from the clone rather than a clear factory-level rejection, making the failure harder to diagnose for integrators/scripts.

• No fund, security, state, or liveness impact - no clone persists, _poolsByAgreement is untouched, and the (agreement, index) salt is unconsumed and reusable on a corrected retry.

Proof of Concept

1. Add the import near the other imports at the top of test/unit/ConfidencePoolFactory.t.sol:

2. Paste the two test functions inside the existing ConfidencePoolFactoryTest contract (its setUp, _scope(), agreementOwner, token, agreement, recovery, and ONE are already defined and cover everything these need):

3. Run from the repo root with traces enabled:

4. Read the trace, not the pass/fail. Both tests pass — the point is what the -vvv trace shows: Clones::cloneDeterministic deploying the clone completes first, and the revert frame (InvalidAmount / ExpiryTooFar) appears underneath it, emitted by the newly-deployed pool rather than by the factory you called. That ordering, deployment gas spent, then a downstream pool-level revert, is the finding.

Recommended Mitigation

Mirror the two missing bounds in createPool before the clone is deployed, so invalid input is rejected cheaply with a clear factory-level error. Keep the initializer checks as defense-in-depth (the pool must stay safe when initialized through any path). This requires exposing ExpiryTooFar / InvalidAmount (or equivalent named errors) on IConfidencePoolFactory.