Root + Impact
Description
A ConfidencePool stores a list of covered accounts and one Agreement address. Once the pool observes that the Agreement has moved past its initial state, the pool locks its local scope so the sponsor cannot remove any covered account.
The Agreement has a separate commitment period. The Agreement owner cannot reduce its scope during this period, but the restriction ends at cantChangeUntil:
function removeAccounts(string memory caip2ChainId, string[] memory accountAddresses) external onlyOwner {
if (block.timestamp < s_cantChangeUntil) {
revert Agreement__CannotReduceScopeDuringCommitment();
}
}
The pool never checks that the Agreement commitment lasts until the pool expiry. Both initialize() and setExpiry() accept an expiry without comparing it to getCantChangeUntil():
function initialize(
address agreement_,
address stakeToken_,
address safeHarborRegistry_,
address outcomeModerator_,
uint256 expiry_,
uint256 minStake_,
address recoveryAddress_,
address owner_,
address[] calldata accounts
) external initializer {
if (expiry_ < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (expiry_ > type(uint32).max) revert ExpiryTooFar();
agreement = agreement_;
expiry = uint32(expiry_);
_replaceScope(accounts);
}
function setExpiry(uint256 newExpiry) external onlyOwner {
if (expiryLocked) revert ExpiryLocked();
if (newExpiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (newExpiry > type(uint32).max) revert ExpiryTooFar();
expiry = uint32(newExpiry);
}
This allows the following setup:
Agreement A scope: [X, Y]
Agreement commitment: 7 days
Pool scope: [X]
Pool expiry: 30 days
Staker principal: 100 tokens
Pool bonus: 50 tokens
The pool can lock X while X belongs to Agreement A. After day 7, the Agreement owner can remove X from A even though the pool remains active for another 23 days. Removing X deletes its registry link to A:
function unregisterContractForExistingAgreement(address contractAddress) external {
delete s_contractToAgreement[contractAddress];
emit ContractUnregistered(contractAddress, msg.sender);
}
X can then be registered under Agreement B. The pool still lists X in its locked scope, but it never follows X to B. Every state check continues to use the original Agreement A:
function _getAgreementState() internal view returns (IAttackRegistry.ContractState) {
address attackRegistry = safeHarborRegistry.getAttackRegistry();
if (attackRegistry == address(0)) revert InvalidAgreement();
return IAttackRegistry(attackRegistry).getAgreementState(agreement);
}
When X is exploited, Agreement B becomes CORRUPTED, while Agreement A can remain UNDER_ATTACK. The pool moderator cannot correct the outcome because flagOutcome(CORRUPTED) requires the pool's original Agreement A to be corrupted:
if (newOutcome == PoolStates.Outcome.CORRUPTED) {
if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
}
At expiry, claimExpired() sees A as UNDER_ATTACK and resolves the pool as EXPIRED. The staker receives the principal and bonus even though X, which is still shown in the pool's locked scope, was breached under B.
Risk
The design documentation partially acknowledges the stale-reference condition. Its scope section titled "**Scope is a fixed, pool-local commitment" **states that narrowing the Agreement may leave the pool's locked scope referencing accounts that are no longer in that Agreement, but claims that “the pool's own commitment to stakers remains the binding source of truth.”
The implementation does not enforce that guarantee. It preserves X only as scope metadata, while resolution continues to depend exclusively on Agreement A's registry state. Once X is rebound to Agreement B, a breach of X corrupts B—not A—so neither the moderator nor the expiry backstop can apply the economic consequence associated with covering X. Therefore, the stale reference itself is documented, but its consequence incorrect settlrement and loss of the whitehat's pool bounty is not.
Likelihood: Medium
-
The issue is reachable whenever the pool expiry is later than the Agreement commitment.
-
The Agreement owner can remove and rebind X through normal protocol functions after the commitment ends.
-
A wrong payout requires X to be breached after it has been moved to another Agreement.
Impact: Medium
-
The pool's locked coverage becomes ineffective for X.
-
The moderator cannot record the correct CORRUPTED outcome.
-
The entire pool can be paid to stakers instead of the good-faith whitehat.
Proof of Concept
Add the following test to test/unit/ScopeCommitmentDrift.t.sol and run:
forge test --match-contract ScopeCommitmentDriftTest -vv
pragma solidity 0.8.26;
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
contract PerAgreementAttackRegistryMock {
mapping(address agreement => IAttackRegistry.ContractState state) internal states;
function setAgreementState(address agreement, IAttackRegistry.ContractState state) external {
states[agreement] = state;
}
function getAgreementState(address agreement) external view returns (IAttackRegistry.ContractState) {
return states[agreement];
}
}
contract ScopeCommitmentDriftTest is BaseConfidencePoolTest {
function testPoC_LockedScopeCanMoveToAnotherAgreementBeforePoolExpiry() external {
PerAgreementAttackRegistryMock registry = new PerAgreementAttackRegistryMock();
safeHarborRegistry.setAttackRegistry(address(registry));
MockAgreement agreementB = new MockAgreement(address(this));
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
registry.setAgreementState(agreement, IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
assertTrue(pool.scopeLocked());
assertTrue(pool.isAccountInScope(DEFAULT_SCOPE_ACCOUNT));
vm.warp(BASE_TIMESTAMP + 7 days);
agreementContract.setContractInScope(DEFAULT_SCOPE_ACCOUNT, false);
agreementB.setContractInScope(DEFAULT_SCOPE_ACCOUNT, true);
registry.setAgreementState(address(agreementB), IAttackRegistry.ContractState.CORRUPTED);
assertTrue(pool.isAccountInScope(DEFAULT_SCOPE_ACCOUNT), "pool still promises to cover X");
assertFalse(agreementContract.isContractInScope(DEFAULT_SCOPE_ACCOUNT), "A no longer covers X");
assertTrue(agreementB.isContractInScope(DEFAULT_SCOPE_ACCOUNT), "B now covers X");
assertEq(uint256(registry.getAgreementState(agreement)), uint256(IAttackRegistry.ContractState.UNDER_ATTACK));
assertEq(
uint256(registry.getAgreementState(address(agreementB))), uint256(IAttackRegistry.ContractState.CORRUPTED)
);
vm.prank(moderator);
vm.expectRevert(IConfidencePool.InvalidOutcome.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
vm.warp(pool.expiry());
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimExpired();
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.EXPIRED));
assertEq(token.balanceOf(alice) - aliceBefore, 150 * ONE, "staker receives stake plus bonus");
assertEq(token.balanceOf(attacker), 0, "whitehat receives no pool bounty");
}
}
The test passes and shows that the staker receives all 150 tokens while the whitehat receives 0.
Recommended Mitigation
Require the Agreement commitment to cover the complete pool lifetime:
uint256 cantChangeUntil = IAgreement(agreement).getCantChangeUntil();
if (cantChangeUntil < poolExpiry) revert AgreementCommitmentTooShort();
