BattleChain Confidence Pools

Description

• Normal behavior: docs/DESIGN.md §11 explicitly documents safeHarborRegistry (and transitively IAttackRegistry) as a trusted, protocol-DAO-controlled singleton, placed out of the adversarial model on purpose. This lets the contract safely treat registry calls as non-adversarial.

• The issue: there is no equivalent statement anywhere in the docs about agreement, even though agreement is the only external contract the pool calls for both identity (IAgreement(agreement).owner(), gating pool creation) and scope membership (IAgreement(agreement).isContractInScope(account), gating _replaceScope). The code's own construction treats agreement as untrusted — it is called with no reentrancy guard, mid-loop, in exactly the place a genuinely trusted dependency would not need one — but nothing confirms whether that's a deliberate conservative stance or an oversight.

Whether agreement is guaranteed to be a single canonical, audited BattleChain implementation (in which case isContractInScope is presumably a plain storage read with no reentrancy surface, and the related reentrancy finding is unreachable in production) or whether Safe Harbor's self-service, per-protocol opt-in model lets each protocol supply its own agreement contract (in which case treating it as untrusted, attacker-reachable code is the correct and necessary posture) is not stated anywhere in the in-scope code or docs.

Risk

Likelihood:

• This is reported as an open question for the sponsors rather than a confirmed defect, since it could not be resolved from the code in scope — lib/battlechain-safe-harbor-contracts is an empty submodule placeholder in this checkout, so the actual IAgreement implementation could not be inspected.

• If agreement turns out to be arbitrary, sponsor-authored bytecode, the likelihood of the associated reentrancy paths (Findings 1 and 4) being live in production is effectively certain for any protocol that self-registers a custom agreement contract.

Impact:

• If agreement can be arbitrary bytecode, this elevates the associated reentrancy findings from theoretical to live Medium-severity issues, since the trust boundary the rest of the documented model relies on (trusted registry, untrusted-by-default everything else) would then also need to explicitly extend to agreement.

• If agreement is guaranteed canonical, the associated findings are effectively moot/informational, since a plain storage read has no reentrancy surface to exploit.

Proof of Concept

Not applicable as an exploit PoC — this finding is a documentation/trust-model gap, not a directly triggerable bug. Its resolution determines the real-world exploitability of the separately-reported reentrancy findings (see Finding 1 and the createPool reentrancy finding), which do have concrete exploit paths contingent on the answer to the question raised here.

Recommended Mitigation

Regardless of the answer, add one sentence to docs/DESIGN.md alongside §11 stating the trust level of agreement explicitly, the same way it already does for the registry:

• If agreement can be arbitrary per-protocol bytecode: treat the related reentrancy findings as confirmed-Medium and apply their fixes (add nonReentrant to the affected entrypoints).

• If agreement is guaranteed to be canonical BattleChain-deployed bytecode only: downgrade the related reentrancy findings to informational and record the reasoning in the docs so future reviewers don't re-flag the same code path.

Ask for sponsors: is IAgreement guaranteed to be canonical BattleChain-deployed bytecode for every registered agreement, or can a Safe Harbor participant supply arbitrary code at that address?