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

Scope lock rollback after reverting observation allows Post-Request Scope Replacement

Author Revealed upon completion

Description

Normally, a pool sponsor can update the pool scope only while the registry is
in the staging states: NOT_DEPLOYED or NEW_DEPLOYMENT. Once the pool
observes any other state, such as ATTACK_REQUESTED, the scope should be
permanently locked so existing stakers keep the coverage they originally accepted.

The issue is that scopeLocked is set inside _observePoolState(), but both
setPoolScope() and pokeRiskWindow() can revert immediately after calling
it. Because a revert rolls back all state changes in the transaction, the
lock is discarded. Later, when the registry returns to NOT_DEPLOYED, the
sponsor can change scope even though the pool already observed
ATTACK_REQUESTED

function setPoolScope(address[] calldata accounts) external onlyOwner {
// @> This can set scopeLocked = true when registry state is ATTACK_REQUESTED
_observePoolState();
​
// @> This revert rolls back the scopeLocked write above
if (scopeLocked) revert ScopePostLockImmutable();
​
_replaceScope(accounts);
}
​
function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
​
// @> This can set scopeLocked = true when registry state is ATTACK_REQUESTED
_observePoolState();
​
// @> ATTACK_REQUESTED does not set riskWindowStart or riskWindowEnd,
// @> so this revert rolls back the scopeLocked write
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
) {
// @> Correctly attempts to lock scope after leaving staging states
scopeLocked = true;
emit ScopeLocked(block.timestamp);
}
​
if (riskWindowStart == 0 && _isActiveRiskState(state)) {
_markRiskWindowStart();
}
​
if (riskWindowEnd == 0 && _isTerminalState(state)) {
_markRiskWindowEnd();
}
}

Likelihood:

  1. The sponsor can regain scope mutability after the registry returns to
    NOT_DEPLOYED / NEW_DEPLOYMENT, even though the pool already observed a
    state that should have closed the sponsor's scope-control window.

  2. During the same ATTACK_REQUESTED period, a sponsor's rejected
    setPoolScope() call also reverts after temporarily setting scopeLocked,
    causing the lock to be rolled back.

Impact:

  • Existing stakers can remain deposited while the pool's published coverage
    scope is replaced, weakening the protocol's stated guarantee that stakers are
    exposed only to the scope they signed up for once the registry leaves staging.

  • The sponsor can regain scope mutability after the registry returns to
    NOT_DEPLOYED / NEW_DEPLOYMENT, even though the pool already observed a
    state that should have closed the sponsor's scope-control window.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
​
contract ScopeLockRollbackTest is BaseConfidencePoolTest {
address internal constant SECOND_SCOPE_ACCOUNT = address(0xBEEF);
​
function testRevertingObservationLetsSponsorChangeScopeAfterRegistryRewind() public {
// Alice stakes while the pool is still in the normal staging state.
_stake(alice, 100 * ONE);
​
// The sponsor prepares another valid agreement scope account.
agreementContract.setContractInScope(SECOND_SCOPE_ACCOUNT, true);
​
// The registry leaves the sponsor's scope-mutation window.
// ATTACK_REQUESTED is outside NOT_DEPLOYED / NEW_DEPLOYMENT,
// so observing it should permanently lock the pool scope.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.ATTACK_REQUESTED);
​
// pokeRiskWindow observes ATTACK_REQUESTED and temporarily sets scopeLocked,
// but then reverts because ATTACK_REQUESTED does not set riskWindowStart/end.
// The revert rolls back scopeLocked.
vm.expectRevert(IConfidencePool.RiskWindowNotReached.selector);
pool.pokeRiskWindow();
assertFalse(pool.scopeLocked(), "poke revert rolled back scope lock");
​
address[] memory replacementScope = new address[](1);
replacementScope[0] = SECOND_SCOPE_ACCOUNT;
​
// setPoolScope has the same issue. It observes ATTACK_REQUESTED,
// temporarily sets scopeLocked, then reverts because scope is locked.
// The revert again rolls back the latch.
vm.expectRevert(IConfidencePool.ScopePostLockImmutable.selector);
pool.setPoolScope(replacementScope);
assertFalse(pool.scopeLocked(), "setPoolScope revert rolled back scope lock");
​
// The trusted registry later returns to NOT_DEPLOYED, for example after
// the attack request is rejected.
attackRegistry.setAgreementState(IAttackRegistry.ContractState.NOT_DEPLOYED);
​
// Because the previous observations reverted, scopeLocked is still false.
// The sponsor can now replace the pool scope while Alice's stake remains.
pool.setPoolScope(replacementScope);
​
assertFalse(pool.isAccountInScope(DEFAULT_SCOPE_ACCOUNT), "old scope removed");
assertTrue(pool.isAccountInScope(SECOND_SCOPE_ACCOUNT), "new scope installed");
assertEq(pool.eligibleStake(alice), 100 * ONE, "existing stake remains under changed scope");
}
}

Recommended Mitigation

Avoid writing scopeLocked and then reverting in the same call path.

One approach is to make observation of a post-staging state persist even when no
risk window starts:

function pokeRiskWindow() external {
if (outcome != PoolStates.Outcome.UNRESOLVED) return;
​
bool wasScopeLocked = scopeLocked;
_observePoolState();
​
// Preserve a newly sealed scope lock even when ATTACK_REQUESTED does not
// open the risk window.
if (!wasScopeLocked && scopeLocked) return;
​
if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
}

For setPoolScope(), avoid reverting after newly sealing scope. The function
should either seal and return without changing scope, or use a separate
non-reverting observation path before enforcing the scope-change restriction.

Also add a regression test for the ATTACK_REQUESTED -> NOT_DEPLOYED rewind
path to ensure the sponsor cannot replace scope after the pool has already
observed ATTACK_REQUESTED.

Support

FAQs

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

Give us feedback!