Rejected Attack Requests Reopen Scope for Existing Stake

Severity: Medium

Description

ConfidencePool intends its pool-local scope to become a permanent commitment
as soon as the associated BattleChain agreement leaves pre-attack staging.
Revision 58e8ba4ce3f3277866e4926f3140e597f9554a1e does not preserve that history.
It derives the lock from the registry's current state and persists it only when
the observing pool transaction succeeds.

During ATTACK_REQUESTED, both scope-specific observation paths can set
scopeLocked and then revert. The EVM rolls the lock write back with the rest of
the transaction. If the registry moderator subsequently rejects the request,
the supported registry deletes the agreement state and reports
NOT_DEPLOYED again. The pool owner can then replace the scope even though a
staker's principal remains deposited from before the request.

The immediate impact is loss of integrity of the scope commitment on which
stakers relied. The changed commitment is also economically meaningful: in a
later approved attack cycle, a breach of a replacement-scope account can
correctly lead the moderator to flag the pool CORRUPTED, after which the
existing principal follows the full-pool recovery or bounty path. Scope
replacement alone does not transfer funds, and the later loss requires several
protocol events, so I assess this as Medium rather than High.

I reviewed revision 58e8ba4ce3f3277866e4926f3140e597f9554a1e
and reproduced the complete state transition locally with Foundry. The focused
test passed and demonstrated replacement of the scope while 100e18 of the
original staker's principal remained eligible, followed by a later bad-faith
CORRUPTED sweep. I did not test a public or live deployment. No fixing
revision was available at the time of review.

Vulnerability Details

We begin with a staker depositing against scope A while the registry is in
pre-attack staging. stake calls _observePoolState, but pre-staging is
deliberately excluded from the lock condition, so scopeLocked remains false.
This is consistent with the current design even after stake exists.

The agreement then enters ATTACK_REQUESTED, the first post-staging state.
At this point the documented permanent commitment should be established. If
the sponsor tries setPoolScope(scopeB), we reach _observePoolState, write
scopeLocked = true, return to setPoolScope, and immediately revert because
that same flag is true. The revert erases the only historical record that the
pool saw ATTACK_REQUESTED.

The permissionless pokeRiskWindow function does not repair this edge
(src/ConfidencePool.sol:649-658):

ATTACK_REQUESTED is neither an active-risk state nor a terminal state.
Therefore pokeRiskWindow also writes the lock and then reverts with
RiskWindowNotReached, rolling the lock back. There is no scope-only call that
can successfully preserve this observation.

The upstream registry makes this transient state important. Its ordinary
moderator rejection path expressly moves an agreement from
ATTACK_REQUESTED back to NOT_DEPLOYED by deleting its state
(lib/battlechain-safe-harbor-contracts/src/AttackRegistry.sol:350-376):

After this normal transition, the pool sees only NOT_DEPLOYED. We can now
call setPoolScope(scopeB) as the sponsor. _observePoolState does not set the
lock, the stale Boolean is still false, and execution reaches
_replaceScope. That function clears every old membership bit and installs the
new array wholesale (src/ConfidencePool.sol:749-775):

The replacement accounts must still be in the agreement's own scope, which
limits arbitrary input but does not preserve the pool commitment. A pool may
cover only a subset of agreement accounts, and changing that subset changes
which breach surfaces the existing staker underwrote.

The resulting state is concrete: scope A is gone, scope B is committed, the
original staker's eligibleStake is unchanged, and the registry has no record
visible to the pool that it previously left staging. A subsequent request can
be registered through the supported registry workflow, approved into
UNDER_ATTACK, and later marked CORRUPTED.

At settlement, the moderator uses the published scope as the binding audit
trail. flagOutcome snapshots all eligible stake into corruptedReserve for a
CORRUPTED result, and claimCorrupted transfers the pool balance to the
recovery address in the bad-faith case (src/ConfidencePool.sol:341-362 and
:408-425). Thus the stale lifecycle latch can affect the disposition of
principal deposited against a different commitment.

Risk

The strongest realistic path uses legitimate sponsor permissions and an
ordinary registry decision rather than compromised infrastructure:

1. A passive staker deposits against scope A during pre-staging.

2. The agreement reaches ATTACK_REQUESTED.

3. No successful pool interaction occurs while that request is pending.

4. The registry moderator rejects the request normally.

5. Before the staker withdraws, the sponsor replaces scope A with scope B.

6. The sponsor requests attack mode again and the registry moderator approves
   it through the ordinary workflow.

7. A later breach involves scope B, and the pool moderator correctly applies
   the now-published commitment when selecting CORRUPTED.

The third step is the main reliability constraint. A successful stake, bonus
contribution, or withdraw call during ATTACK_REQUESTED carries the
scopeLocked write through to transaction completion and permanently closes
the path. In particular, withdraw permits ATTACK_REQUESTED; an alert staker
who exits during the request both removes their principal and persists the
lock. The exploit therefore targets a pool whose stakers are passive during
the pending request, which is plausible for escrow intended to sit through a
protocol lifecycle but is not guaranteed.

After rejection, withdrawal is available again. The sponsor must order the
scope replacement before the affected staker exits. This is a public
transaction-ordering condition, not an irreversible trap. A staker monitoring
the rejection can protect themselves by withdrawing first, although the
documented model should not require continuous monitoring to preserve a
permanent commitment.

The sponsor also cannot manufacture the final loss solely through
setPoolScope. The replacement list is constrained to agreement accounts, a
new request must pass ordinary registry approval, a later relevant breach must
occur, and the trusted outcome moderator must choose CORRUPTED according to
the replacement scope. Scope mutation itself transfers no tokens. These
dependencies reduce likelihood and make a High rating inappropriate.

Conversely, owner-only access does not reduce the issue to trusted-admin
behavior. The privilege delta is precisely temporal: the sponsor has scope
authority in pre-staging and is expressly supposed to lose it permanently
after the first post-staging state. The bug restores a permission that should
have expired and lets that restored permission alter another actor's escrow
terms. The immediate commitment-integrity violation is deterministic, while
the fund consequence is contingent but can encompass all principal in the
affected pool. That combination supports Medium severity.

Proof of Concept

The included Foundry test uses the repository's normal pool fixture and a
registry-state mock to isolate the lifecycle edge. The mock transitions mirror
the concrete upstream states; the report above separately shows the production
registry's deletion-based rejection implementation.

We first stake 100e18 for Alice against the default scope. We then move to
ATTACK_REQUESTED and prove that both setPoolScope and pokeRiskWindow
revert after their temporary lock write, leaving scopeLocked == false. After
moving to NOT_DEPLOYED, we replace the scope and assert that Alice's full
eligibleStake remains. Finally, we run a later
UNDER_ATTACK -> CORRUPTED cycle, flag bad-faith corruption, and show the full
100e18 reaching the recovery address.

The bundle is intended for a clean checkout of the vulnerable revision. Place
the entire rejected-attack-request-reopens-scope directory at
test/rejected-attack-request-reopens-scope, then run:

No live deployment is contacted and the test changes no persistent external
state. Representative output is:

Recommended Mitigation

The invariant to restore is historical: once any pool call observes a registry
state other than NOT_DEPLOYED or NEW_DEPLOYMENT, that observation must
commit scopeLocked = true in a successful transaction, and no later registry
rewind may make scope mutable again.

The important implementation constraint is that a function cannot both
persist the new latch and revert. A minimal safe pattern is to make the first
post-staging scope mutation attempt a successful no-op after it seals the lock;
subsequent attempts can revert because the latch was committed by an earlier
transaction. pokeRiskWindow should likewise return successfully when it has
just sealed scope in ATTACK_REQUESTED, even though no risk window opened:

This preserves the existing mutation window and makes the ScopeLocked event
observable, but the successful no-op should be documented clearly because a
caller must use ScopeUpdated, not transaction success alone, to infer a scope
change. A cleaner API variant is a non-reverting syncPoolState()/sealScope()
operation plus explicit status reporting; however, merely adding such an
optional function is insufficient unless every path that observes the first
post-staging state preserves the latch.

A structurally stronger alternative is to lock scope on the first successful
stake, just as expiry is locked. That makes the staker's deposited-against
scope immutable without depending on registry history, but it intentionally
narrows the sponsor's documented pre-staging mutation window and should be
adopted as a product-level invariant rather than a silent patch.

Regression coverage should include:

• ATTACK_REQUESTED -> reject -> NOT_DEPLOYED with no intervening pool call,
  proving scope cannot be replaced while stake remains;

• a first setPoolScope attempt during ATTACK_REQUESTED, proving the
  transaction preserves scopeLocked without replacing scope;

• pokeRiskWindow during ATTACK_REQUESTED, proving it seals scope without
  opening riskWindowStart or reverting;

• rejection after successful stake, bonus, and withdrawal interactions during
  the request, proving all observation paths retain the same one-way invariant;

• ordinary scope replacement in both allowed pre-staging states, proving the
  intended setup window still works.

Complete Foundry PoC