BattleChain Confidence Pools

Severity: Low.DoS only, no fund loss
Affected: ConfidencePoolFactory.createPool at src/ConfidencePoolFactory.sol:67-101

TL;DR

createPool writes _poolsByAgreement[agreement].push(pool) after the external pool.initialize() call (which makes external subcalls to SafeHarborRegistry.isAgreementValid and Agreement.isContractInScope). If either subcall re-enters createPool, a CREATE2 salt collision DoSes one of the two deployments. The team suppressed the aderyn warning at line 89. No fund loss possible.

Description

createPool computes a deterministic CREATE2 salt from the agreement address and the current _poolsByAgreement[agreement].length:

initialize makes two external subcalls:

1. IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_) at ConfidencePool.sol:200

2. IAgreement(agreement).isContractInScope(account) at ConfidencePool.sol:768 (inside _replaceScope)

The array _poolsByAgreement[agreement] is not updated until after initialize returns. If either subcall re-enters createPool, the re-entrant call reads the same stale _poolsByAgreement[agreement].length, computes the same salt, and deploys a clone at the SAME deterministic address ; causing a collision. The aderyn-fp-next-line(reentrancy-state-change) suppression at line 89 confirms the team was aware of the pattern.

Attack Path

1. A SafeHarborRegistry or Agreement contract has a callback that re-enters ConfidencePoolFactory.createPool during isAgreementValid or isContractInScope

2. The re-entrant createPool reads _poolsByAgreement[agreement].length ; which has NOT been incremented yet

3. The same salt is computed → cloneDeterministic deploys at the same address

4. CREATE2 collision: the second deployment fails (address already occupied) or overwrites the first

5. One pool creation is DoSed

Scope Eligibility

ConfidencePoolFactory.sol is listed in the contest scope (589 nSLOC total, includes both pool and factory). The createPool function is in the factory's deployed bytecode. This finding does not rely on any out-of-scope dependency ; the CEI pattern is purely within the factory code. The aderyn-fp-next-line(reentrancy-state-change) suppression at line 89 confirms the team was aware of the pattern but chose to suppress the warning rather than fix the ordering.

Severity Calibration

CodeHawks severity rules: Low findings are "issues that are not exploitable under normal circumstances but represent poor coding practices or deviations from best practices that could theoretically lead to issues."

This finding: the CEI violation is structurally present (state write after external call) but currently unexploitable because the only downstream external calls (isAgreementValid, isContractInScope) are view functions on trusted contracts. If a future upgrade to either dependency introduces a callback, the re-entrancy window would open. Severity: Low.

Known-Issue Distinction

No prior audit reports or PRs flag the CEI violation in createPool. The aderyn-fp-next-line(reentrancy-state-change) suppression at line 89 indicates the team was aware of the aderyn warning but chose to suppress it. The DESIGN.md does not discuss CEI ordering in factory functions.

This finding is novel: it identifies a structural CEI violation that the static analysis tool flagged and the team suppressed.

Risk

1. A SafeHarborRegistry or Agreement contract implements a callback that re-enters ConfidencePoolFactory.createPool during isAgreementValid or isContractInScope. Likelihood: very low ; both are protocol-level contracts controlled by the BattleChain DAO and are currently view/pure functions with no expected callbacks.

2. The callback triggers during the window between pool.initialize() and _poolsByAgreement.push() ; this is the CEI window where _poolsByAgreement[agreement].length is stale. Likelihood: deterministic if condition 1 is met.

3. The re-entrant call computes the same CREATE2 salt, causing a collision. Likelihood: deterministic if conditions 1-2 are met.

Historical occurrence: No similar CEI exploit has been observed on this contract (code is pre-launch). CEI violations have been flagged in numerous other audits (Trail of Bits, Spearbit) as Low/Informational findings, including cases with CREATE2 salt computation.

Denial of service on pool creation. No direct fund loss (0 ETH) ; the affected pool has no stakers yet. The attacker's only cost is the gas for a failed createPool transaction (~0.02 ETH at 100 gwei on L2). The victim sponsor must redeploy with a new index, costing an additional ~0.02 ETH. The factory remains fully operational. The factory owner can increase the index by creating a dummy pool to advance _poolsByAgreement[agreement].length, then retry.

Proof of Concept

The CEI pattern is visible in the source:

Verify the contract compiles and the CEI pattern exists:

Recommended Mitigation

Move _poolsByAgreement[agreement].push(pool) before initialize:

The pool address is deterministic from the salt (which includes the pre-increment length), so the push order does not affect address computation ; it only prevents re-entrancy from seeing a stale array length.