FoundrySolidityLayer 2
7.25 ETH
Submission Details
Impact: high
Likelihood: low

`setPoolScope` can be permanently bricked by a reverting call

Author Revealed upon completion

Root + Impact

Description

The setPoolScope function allows the pool owner to update the list of covered contracts before the scope is locked. The function correctly calls _observePoolState() to check and update the scopeLocked flag. However, the check if (scopeLocked) revert ScopePostLockImmutable() occurs after the call to _observePoolState().

A pool owner can call setPoolScope with an empty array []. This call will pass the initial scopeLocked check, but the subsequent internal call to _replaceScope will revert because an empty scope is disallowed. Crucially, the call to _observePoolState() will have already executed. If the underlying agreement became active in the meantime, this will set scopeLocked to true. From this point forward, any legitimate attempt to set the scope will be permanently blocked, bricking the pool's configuration.

// Root cause in the codebase with @> marks to highlight the relevant section
function setPoolScope(address[] calldata accounts) external onlyOwner {
// aderyn-ignore-next-line(unchecked-return)
@> _observePoolState();
@> if (scopeLocked) revert ScopePostLockImmutable();
_replaceScope(accounts);
}
function _replaceScope(address[] calldata accounts) internal {
@> if (accounts.length == 0) revert EmptyScope();
// ...
}

Risk

Likelihood: Low

  • Reason 1

    • This scenario occurs when the pool owner calls setPoolScope with an empty array of accounts.

  • Reason 2

    • This action must be taken during the specific window where the scope is not yet locked, but the underlying agreement has transitioned to an active state.

Impact: High

  • Impact 1

    • The setPoolScope function becomes permanently unusable for the affected pool.

  • Impact 2

    • If the pool was created with an incorrect scope, the owner loses the ability to correct it, rendering the pool useless and potentially trapping any contributed bonus funds.

Proof of Concept

// Add this test to a relevant test file, like `ConfidencePool.t.sol`
function test_PoC_setPoolScope_PermanentDoS() public {
// 1. Pre-condition: The pool's scope is not yet locked.
assertFalse(pool.scopeLocked(), "Scope should not be locked initially");
// 2. Setup: The underlying agreement becomes active, which will cause _observePoolState
// to set scopeLocked = true.
vm.prank(BATTLECHAIN_REGISTRY_OWNER);
attackRegistry.setAgreementState(address(agreement), IAttackRegistry.ContractState.UNDER_ATTACK);
// 3. Attack: The owner calls setPoolScope with an invalid (empty) accounts array.
// The transaction reverts as expected, but not before the state is poisoned.
address[] memory emptyScope;
vm.prank(sponsor);
vm.expectRevert(IConfidencePool.EmptyScope.selector);
pool.setPoolScope(emptyScope);
// 4. Verification: The scope is now permanently locked due to the side effect in the
// reverting call.
assertTrue(pool.scopeLocked(), "Scope should now be locked");
// 5. Consequence: A subsequent, valid call to setPoolScope now fails,
// proving the function is permanently bricked.
address[] memory validScope = new address;
validScope[0] = address(0x123);
vm.prank(sponsor);
vm.expectRevert(IConfidencePool.ScopePostLockImmutable.selector);
pool.setPoolScope(validScope);
log("PoC successful: setPoolScope is permanently bricked.");
}

Recommended Mitigation

Move the if (scopeLocked) check to occur before calling _observePoolState(). This ensures that a call that is destined to fail cannot change the scopeLocked state.

/// @inheritdoc IConfidencePool
// aderyn-ignore-next-line(centralization-risk)
function setPoolScope(address[] calldata accounts) external onlyOwner {
+ if (scopeLocked) revert ScopePostLockImmutable();
// aderyn-ignore-next-line(unchecked-return)
_observePoolState();
- if (scopeLocked) revert ScopePostLockImmutable();
_replaceScope(accounts);
}

Support

FAQs

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

Give us feedback!