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

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.

Updates

Lead Judging Commences

inallhonesty Lead Judge 2 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Sponsor can atomically self-trigger ATTACK_REQUESTED and roll back scopeLocked via a reverting setPoolScope/pokeRiskWindow, reopening scope after a DAO rejection

Impact - Low Nothing moves when the scope is swapped, and the state afterward is pre-attack, so any staker who notices can withdraw in full. _replaceScope emits ScopeUpdated, which puts the change on-chain where it can be seen. Real loss needs a restarted attack cycle that breaches one of the substituted accounts with the original staker still in the pool, at which point their principal funds a CORRUPTED payout for coverage they never chose. Likelihood - Medium The step the sponsor cannot force is the moderator rejecting the attack request, though rejections are an ordinary registry operation. The rest they can arrange. Neither observation path can seal the lock in this state, and pausing the pool shuts the deposit routes, leaving only a full-exit withdraw to do it. So reaching the mutable-scope condition is not hard; what holds the severity down is how much has to go wrong afterward for anyone to actually lose money.

Support

FAQs

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

Give us feedback!