Submission Details
Impact:
Likelihood:
A rejected attack request can erase the permanent scope lock and reopen scope changes
Author Revealed upon completion
## Description
The pool promises to lock its scope permanently on the first interaction that observes any
registry state other than `NOT_DEPLOYED` or `NEW_DEPLOYMENT`. Accordingly,
`_observePoolState()` sets `scopeLocked = true` when it observes `ATTACK_REQUESTED`.
However, `ATTACK_REQUESTED` is neither an active-risk state nor a terminal state. Therefore the
same observation does not set `riskWindowStart` or `riskWindowEnd`. The permissionless
`pokeRiskWindow()` function then sees both timestamps still equal to zero and reverts with
`RiskWindowNotReached`. EVM revert semantics undo the earlier `scopeLocked = true` write and its
`ScopeLocked` event.
The owner-only `setPoolScope()` path cannot preserve the historical lock either. While the registry
is `ATTACK_REQUESTED`, it calls `_observePoolState()`, temporarily sets the lock, and then reverts
with `ScopePostLockImmutable`; that revert also rolls back the lock.
The upstream registry contains a normal rejection flow. `rejectAttackRequest()` requires the live
state to be `ATTACK_REQUESTED`, clears the registration, and makes `_getAgreementState()` return
`NOT_DEPLOYED`. If no successful pool operation persisted the lock before this rejection, the pool
has forgotten that it observed a post-staging state. The sponsor can then call `setPoolScope()` and
replace the scope attached to existing stake.
The replacement account must pass `_replaceScope()` and therefore must already be in the
underlying Agreement's BattleChain scope. This issue does not let the sponsor insert an arbitrary
unrelated address, directly choose a CORRUPTED outcome, or bypass the protocol moderator.
## Root + Impact
The root cause is that `pokeRiskWindow()` decides whether useful observation work occurred solely
from the two risk timestamps. It does not consider the independently meaningful transition of
`scopeLocked` from `false` to `true`.
```solidity
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
@> _observePoolState();
// ATTACK_REQUESTED sets neither timestamp, even though it temporarily locks scope.
@> if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
}
function _observePoolState() internal returns (IAttackRegistry.ContractState state) {
state = _getAgreementState();
if (
!scopeLocked && state != IAttackRegistry.ContractState.NOT_DEPLOYED
&& state != IAttackRegistry.ContractState.NEW_DEPLOYMENT
) {
@> scopeLocked = true;
@> emit ScopeLocked(block.timestamp);
}
// ATTACK_REQUESTED satisfies neither branch.
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}
```
The production dependency then provides the rewind that makes the rolled-back observation
security-relevant:
```solidity
function rejectAttackRequest(address agreementAddress, bool slashBond)
external
onlyRegistryModerator
{
ContractState currentState = _getAgreementState(agreementAddress);
if (currentState != ContractState.ATTACK_REQUESTED) revert AttackRegistry__InvalidState(currentState);
// Clear the Agreement's registered contract mappings, then its AgreementInfo.
@> emit AgreementStateChanged(agreementAddress, ContractState.NOT_DEPLOYED);
@> delete s_agreementInfo[agreementAddress];
// Bond handling omitted.
}
function _getAgreementState(address agreementAddress) internal view returns (ContractState) {
AgreementInfo storage info = s_agreementInfo[agreementAddress];
// ... terminal checks omitted ...
@> if (!info.isRegistered) return ContractState.NOT_DEPLOYED;
// ...
}
```
After rejection, the still-false latch allows the sponsor to replace the commitment:
```solidity
function setPoolScope(address[] calldata accounts) external onlyOwner {
_observePoolState(); // observes NOT_DEPLOYED, so it does not lock
@> if (scopeLocked) revert ScopePostLockImmutable();
@> _replaceScope(accounts);
}
```
Existing stakers can consequently be exposed to a different subset of the Agreement than the one
they inspected. A later breach of a newly substituted account can legitimately lead the moderator
to flag this pool `CORRUPTED`, at which point the full pool balance can be routed according to the
documented CORRUPTED rules. The bug is the broken historical commitment; the later loss is
conditional and is not automatic at scope replacement time.
## Impact
**Medium.** The sponsor can replace the pool-local scope that existing stakers relied
on after the registry has already left staging. If a newly substituted account is later the surface
of a genuine corruption, the moderator can correctly classify the pool as `CORRUPTED`, causing all
stake and bonus to follow the CORRUPTED settlement path.
The scope rewrite does not itself transfer funds. Loss additionally requires a later approved risk
cycle, a breach attributable to the replacement scope, moderator classification, and stakers not
withdrawing before risk begins. These conditions keep the overall finding Low severity.
## Likelihood
**Low.** Exploitation requires all of the following:
- the registry enters `ATTACK_REQUESTED`;
- no successful pool call persists `scopeLocked` while that request is pending;
- the DAO rejects the request through the real `rejectAttackRequest` transition;
- the sponsor replaces the pool scope with another account already present in the Agreement's
BattleChain scope;
- the Agreement is registered and approved again;
- stakers miss their still-open withdrawal opportunity; and
- a later genuine breach is attributable to the substituted account.
A successful stake, bonus contribution, or funded withdrawal during `ATTACK_REQUESTED` persists the
lock, and vigilant stakers can withdraw after rejection. Those defenses materially reduce
likelihood but do not preserve the promised invariant through the dedicated permissionless poke.
## Risk
**Overall risk: Low.**
Impact is meaningful because the pool scope determines which BattleChain accounts the moderator
uses when deciding whether a breach is in scope. Replacing it can change existing stakers' maximum
exposure, and a later valid CORRUPTED settlement can affect the complete pool balance.
Likelihood is low because the attack depends on a narrow and observable lifecycle sequence. The
request must be rejected before any successful pool call persists the lock; the replacement account
must already belong to the Agreement; stakers retain a withdrawal window; the DAO must later
approve another attack cycle; and real corruption plus moderator attribution must occur. Since the
scope rewrite alone neither moves funds nor guarantees the later outcome, Medium severity would
overstate the directness of the loss.
## Proof of Concept
The mock registry supplies the same
state values as the production dependency. The transition it models is exact: production
`AttackRegistry.rejectAttackRequest()` accepts only `ATTACK_REQUESTED`, deletes
`s_agreementInfo`, and production `_getAgreementState()` consequently returns `NOT_DEPLOYED`.
```solidity
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
contract ScopeLockRewindPoC is BaseConfidencePoolTest {
address internal constant RISKY_ACCOUNT = address(0xBAD);
function test_PoC_RejectedAttackRequestReopensPurportedlyPermanentScope() external {
// Alice relies on the initially published scope and supplies principal and bonus.
_stake(alice, 100 * ONE);
_contributeBonus(alice, 20 * ONE);
// The replacement is already part of the underlying Agreement, but not this pool.
agreementContract.setContractInScope(RISKY_ACCOUNT, true);
// The registry leaves staging. Observation writes scopeLocked and then reverts because
// ATTACK_REQUESTED sets neither risk timestamp. The complete transaction rolls back.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.ATTACK_REQUESTED);
address[] memory replacement = new address[](1);
replacement[0] = RISKY_ACCOUNT;
vm.expectRevert(IConfidencePool.RiskWindowNotReached.selector);
pool.pokeRiskWindow();
assertFalse(pool.scopeLocked(), "the supposedly one-way lock did not persist");
// Faithfully model production rejectAttackRequest(): deleting AgreementInfo makes the
// production state getter return NOT_DEPLOYED.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
// The sponsor can now replace the pool-local commitment for Alice's existing position.
pool.setPoolScope(replacement);
assertTrue(pool.isAccountInScope(RISKY_ACCOUNT));
assertFalse(pool.isAccountInScope(DEFAULT_SCOPE_ACCOUNT));
// A later genuine attack involving the replacement scope is classified CORRUPTED by the
// moderator. This does not assume the scope rewrite itself selects the outcome.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
uint256 recoveryBefore = token.balanceOf(recovery);
pool.claimCorrupted();
assertEq(token.balanceOf(recovery) - recoveryBefore, 120 * ONE);
assertEq(token.balanceOf(address(pool)), 0);
}
}
```
The assertion that `scopeLocked()` remains false proves the rollback. The two scope-membership
assertions prove that the existing commitment was replaced, and the final balance assertion
quantifies the conditional downstream exposure. The PoC does not claim that the mock setter is an
attacker capability; it models the exact DAO rejection edge implemented by the production
dependency.
## Recommended Mitigation
Treat a newly sealed scope lock as a successful poke:
```diff
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
+ bool scopeLockedBefore = scopeLocked;
_observePoolState();
- if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
+ if (scopeLocked == scopeLockedBefore
+ && riskWindowStart == 0 && riskWindowEnd == 0) {
+ revert RiskWindowNotReached();
+ }
}
```
Test `ATTACK_REQUESTED -> rejectAttackRequest -> NOT_DEPLOYED` and confirm the lock remains set.
Prize pool breakdown
Total prize
7.25 ETH
nSLOC
5890
ETH / LOC
6
1.25
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.
Rewards Distribution
The contest is complete and the rewards are being distributed.