BattleChain Confidence Pools
Cyfrin
FoundrySolidityLayer 2
7.25 ETH
View all submissions
Previous Next
Submission Details
Impact: medium
Likelihood: low

`pokeRiskWindow()` cannot persist scope lock during `ATTACK_REQUESTED`, allowing scope mutation after a transient non-staging state

Author Revealed upon completion

pokeRiskWindow(), the permissionless function designed to let anyone observe the registry state and seal pool state markers cannot persist scopeLocked = true when the registry is in ATTACK_REQUESTED. The function's internal call to _observePoolState() correctly sets scopeLocked = true (since ATTACK_REQUESTED is not NOT_DEPLOYED or NEW_DEPLOYMENT), but pokeRiskWindow() then reverts with RiskWindowNotReached because ATTACK_REQUESTED is neither an active-risk nor a terminal state, rolling back the scope lock write. If the DAO subsequently rejects the attack request via rejectAttackRequest(), the registry returns to NOT_DEPLOYED, and the sponsor can freely change the pool's scope via setPoolScope(), despite stakers having committed funds against the original scope during a state that DESIGN.md defines as scope-locking.

Description

DESIGN.md establishes a clear scope-locking guarantee:

"The sponsor can update scope freely while the registry is in NOT_DEPLOYED / NEW_DEPLOYMENT (pre-attack staging). Scope locks permanently on the first interaction observing any other state."

The phrase "any other state" explicitly includes ATTACK_REQUESTED, it is neither NOT_DEPLOYED nor NEW_DEPLOYMENT. Under this guarantee, the first interaction that observes ATTACK_REQUESTED should permanently lock the scope.

_observePoolState() correctly implements this guarantee:

// src/ConfidencePool.sol:784-792
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);
}
// ...
}

However, pokeRiskWindow() wraps this call with its own revert guard:

// src/ConfidencePool.sol:649-658
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
_observePoolState();
if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
}

When the registry is in ATTACK_REQUESTED:

  1. _observePoolState() runs. ATTACK_REQUESTED ≠ NOT_DEPLOYED and ≠ NEW_DEPLOYMENT, so it sets scopeLocked = true.

  2. ATTACK_REQUESTED is not an active-risk state (_isActiveRiskState returns false, only UNDER_ATTACK and PROMOTION_REQUESTED qualify), so riskWindowStart stays 0.

  3. ATTACK_REQUESTED is not a terminal state (_isTerminalState returns false, only PRODUCTION and CORRUPTED qualify at line 837-838), so riskWindowEnd stays 0.

  4. Back in pokeRiskWindow(), line 657 checks: riskWindowStart == 0 && riskWindowEnd == 0 → true → reverts with RiskWindowNotReached.

  5. The EVM revert rolls back all state changes in the transaction, including the scopeLocked = true write from step 1.

The scope lock that _observePoolState() correctly set is erased because the calling function reverts.

This matters because the ATTACK_REQUESTED state can be reversed. The AttackRegistry's rejectAttackRequest() function deletes the agreement info, returning the state to NOT_DEPLOYED:

// lib/battlechain-safe-harbor-contracts/src/AttackRegistry.sol:355-376
function rejectAttackRequest(address agreementAddress, bool slashBond) external onlyRegistryModerator {
// ...
emit AgreementStateChanged(agreementAddress, ContractState.NOT_DEPLOYED);
delete s_agreementInfo[agreementAddress];
// ...
}

After rejection, the registry reads NOT_DEPLOYED. Since scopeLocked was never persisted, the sponsor can call setPoolScope():

// src/ConfidencePool.sol:636-641
function setPoolScope(address[] calldata accounts) external onlyOwner {
_observePoolState();
if (scopeLocked) revert ScopePostLockImmutable();
_replaceScope(accounts);
}

_observePoolState() sees NOT_DEPLOYED and does not set scopeLocked. The if (scopeLocked) check passes. The sponsor replaces the pool's scope accounts.

Other functions that call _observePoolState() have varying ability to persist the scope lock during ATTACK_REQUESTED:

Function Can persist lock during ATTACK_REQUESTED? Limitation
pokeRiskWindow() No — reverts RiskWindowNotReached The dedicated permissionless observation function fails
stake() Yes — ATTACK_REQUESTED is not blocked by _assertDepositsAllowed Gated by whenPoolNotPaused; blocked if owner pauses the pool
contributeBonus() Yes — same as stake() Also gated by whenPoolNotPaused
withdraw() Yes — ATTACK_REQUESTED is explicitly allowed Forces the staker to exit entirely, forfeiting bonus claim
setPoolScope() No — sets lock then immediately reverts ScopePostLockImmutable Self-blocks in the same call
claimExpired() Only post-expiry Reverts if block.timestamp < expiry
flagOutcome() Moderator-only Not permissionless

When the pool is paused (a rational owner action during an attack request), the only permissionless path that could persist scope lock is withdraw(), which forces a full exit. No permissionless non-destructive observation path exists during ATTACK_REQUESTED when the pool is paused. This directly contradicts DESIGN.md §8's guarantee that scope locks "on the first interaction observing any other state," since the permissionless observation function cannot fulfill that guarantee during ATTACK_REQUESTED.

Attack Path Under Normal Operations

  1. Stakers deposit via stake() while registry is NOT_DEPLOYED. Scope is unlocked. Stakers commit funds against the current scope.

  2. A whitehat calls requestUnderAttack() on the AttackRegistry. State transitions to ATTACK_REQUESTED.

  3. The pool owner calls pause() (rational behavior during a pending attack review).

  4. A staker calls pokeRiskWindow() to lock scope — the call reverts with RiskWindowNotReached. Scope lock is not persisted.

  5. With the pool paused, stake() and contributeBonus() also revert (whenPoolNotPaused). The only remaining path is withdraw(), which forces a full exit.

  6. The DAO calls rejectAttackRequest(). Registry returns to NOT_DEPLOYED.

  7. The owner calls unpause(), then setPoolScope() with a different set of scope accounts. The call succeeds because scopeLocked is still false.

  8. Existing stakers who did not withdraw and do not actively re-check scope are now exposed to a different pool scope than the one they evaluated when depositing.

Impact Details

DESIGN.md states:

"Once locked, the pool's coverage is fixed even if the sponsor later expands the agreement, so post-stake additions to the underlying agreement do NOT extend this pool's coverage, and stakers' exposure is bounded by what they signed up for at deposit time."

The scope-locking mechanism exists specifically to protect stakers from having their risk exposure changed after they commit funds. The finding breaks this guarantee: stakers deposit against scope [A, B, C], the agreement briefly enters ATTACK_REQUESTED, the request is rejected, and the sponsor changes scope to [A, B, D]. Contract D might have a much higher risk profile than C, but stakers are now insuring it without having consented.

The practical impact is limited by several factors:

  • Stakers can withdraw() during ATTACK_REQUESTED and after the rejection (since withdrawals are open pre-risk). They can re-evaluate and exit if unhappy with the new scope.

However, the deviation from the documented guarantee is clear, and the attack path uses only standard protocol operations.

Recommended Mitigation Steps

The simplest fix is to make pokeRiskWindow() not revert when it successfully observes a non-staging state, even if no risk window markers were set. This allows the scope lock to persist for ATTACK_REQUESTED:

function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
+ bool wasLocked = scopeLocked;
_observePoolState();
- if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
+ if (riskWindowStart == 0 && riskWindowEnd == 0 && !scopeLocked) revert RiskWindowNotReached();
}

With this change, if _observePoolState() set scopeLocked = true (meaning a non-staging state was observed), the function completes successfully and the lock persists, even if no risk window markers were set yet. The revert still fires if the registry is in NOT_DEPLOYED/NEW_DEPLOYMENT (where scopeLocked would remain false), preserving the original behavior for true pre-staging calls.

POC

Create a file at test/audit/PoC_ScopeLockRevert.t.sol and paste the code below, then run with:

forge test --match-path test/audit/PoC_ScopeLockRevert.t.sol -vvv
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
/// @title PoC: pokeRiskWindow() cannot persist scope lock during ATTACK_REQUESTED
/// @notice DESIGN.md S8 says scope locks "on the first interaction observing any other state"
/// (i.e. any state other than NOT_DEPLOYED / NEW_DEPLOYMENT). ATTACK_REQUESTED is
/// "any other state," but pokeRiskWindow() reverts RiskWindowNotReached when it
/// observes ATTACK_REQUESTED, rolling back the scopeLocked = true write.
/// After the DAO rejects the attack request (returning to NOT_DEPLOYED), the sponsor
/// can freely change scope via setPoolScope(), breaking the S8 guarantee.
contract PoC_ScopeLockRevert is BaseConfidencePoolTest {
address internal constant NEW_SCOPE_ACCOUNT = address(0xBEEF);
​
function setUp() public override {
super.setUp();
// Seed the new scope account as in-scope on the agreement so setPoolScope accepts it.
agreementContract.setContractInScope(NEW_SCOPE_ACCOUNT, true);
}
​
function test_scopeChangesAfterAttackRequestedRejection() external {
// =====================================================================
// Step 1: Stakers deposit while registry is NEW_DEPLOYMENT.
// They evaluate and commit against scope = [DEFAULT_SCOPE_ACCOUNT].
// =====================================================================
_stake(alice, 100 ether);
_stake(bob, 50 ether);
​
address[] memory originalScope = pool.getScopeAccounts();
assertEq(originalScope.length, 1);
assertEq(originalScope[0], DEFAULT_SCOPE_ACCOUNT);
assertFalse(pool.scopeLocked());
​
// =====================================================================
// Step 2: Registry transitions to ATTACK_REQUESTED.
// This is "any other state" per DESIGN.md S8 — scope should lock.
// =====================================================================
attackRegistry.setAgreementState(IAttackRegistry.ContractState.ATTACK_REQUESTED);
​
// =====================================================================
// Step 3: Owner pauses the pool (rational during a pending attack review).
// =====================================================================
pool.pause();
​
// =====================================================================
// Step 4: A staker tries pokeRiskWindow() to lock scope — REVERTS.
// _observePoolState() sets scopeLocked = true, but
// pokeRiskWindow() reverts RiskWindowNotReached because
// ATTACK_REQUESTED is neither active-risk nor terminal.
// The revert rolls back scopeLocked = true.
// =====================================================================
vm.prank(alice);
vm.expectRevert(IConfidencePool.RiskWindowNotReached.selector);
pool.pokeRiskWindow();
​
// scopeLocked is still false — the revert rolled it back.
assertFalse(pool.scopeLocked());
​
// =====================================================================
// Step 5: With pool paused, stake() and contributeBonus() also revert.
// No permissionless non-destructive path can persist the lock.
// =====================================================================
token.mint(carol, 10 ether);
vm.startPrank(carol);
token.approve(address(pool), 10 ether);
vm.expectRevert(IConfidencePool.PoolPaused.selector);
pool.stake(10 ether);
vm.stopPrank();
​
// withdraw() works but forces a full exit — not a "lock scope" path.
// (We don't call it here because we want to show stakers remain committed.)
​
// =====================================================================
// Step 6: DAO rejects the attack request. Registry returns to NOT_DEPLOYED.
// =====================================================================
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
​
// =====================================================================
// Step 7: Owner unpauses, then changes scope.
// setPoolScope() succeeds because scopeLocked is still false.
// =====================================================================
pool.unpause();
​
address[] memory newScope = new address[](1);
newScope[0] = NEW_SCOPE_ACCOUNT;
pool.setPoolScope(newScope);
​
// =====================================================================
// Step 8: Verify — scope has changed under existing stakers' feet.
// Alice and Bob deposited against [DEFAULT_SCOPE_ACCOUNT] but
// are now insuring [NEW_SCOPE_ACCOUNT].
// =====================================================================
address[] memory currentScope = pool.getScopeAccounts();
assertEq(currentScope.length, 1);
assertEq(currentScope[0], NEW_SCOPE_ACCOUNT);
​
// Original scope account is gone.
assertTrue(currentScope[0] != DEFAULT_SCOPE_ACCOUNT);
​
// Stakers are still committed — their funds are in the pool.
assertEq(pool.eligibleStake(alice), 100 ether);
assertEq(pool.eligibleStake(bob), 50 ether);
​
// scopeLocked is STILL false — scope can be changed again.
assertFalse(pool.scopeLocked());
}
​
}

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!