Root + Impact
Description
-
ConfidencePool is intended to lock a pool-local list of BattleChain accounts once the registry leaves pre-attack staging. After that point, changes to the underlying Safe Harbor agreement, including scope narrowing, should not change what the pool covers. When an in-scope account is corrupted, the moderator should be able to flag the pool as CORRUPTED and route the pool through the corrupted settlement path.
-
The design documentation acknowledges the underlying narrowing condition: a locked pool scope may keep referencing accounts no longer present in the original agreement, but the pool-local commitment remains the binding source of truth. The implementation does not preserve that commitment for settlement. It resolves corruption only by reading the original immutable agreement address. BattleChain agreements can remove a BattleChain account after the commitment window, which unregisters that account from the original agreement and allows it to be rebound under another agreement. When the exact account still locked in the pool is later corrupted under the new binding agreement, the original agreement can remain non-corrupted, so flagOutcome(CORRUPTED) reverts and claimExpired() can incorrectly return stake plus bonus to stakers.
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
if (newOutcome == PoolStates.Outcome.SURVIVED) {
if (goodFaith_ || attacker_ != address(0)) {
revert InvalidGoodFaithParams();
}
if (state != IAttackRegistry.ContractState.PRODUCTION && state != IAttackRegistry.ContractState.CORRUPTED) {
revert InvalidOutcome();
}
} else if (newOutcome == PoolStates.Outcome.CORRUPTED) {
if (!goodFaith_) {
if (attacker_ != address(0)) revert InvalidGoodFaithParams();
} else {
if (attacker_ == address(0)) revert InvalidGoodFaithParams();
}
@> if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
} else {
revert InvalidOutcome();
}
}
function _getAgreementState() internal view returns (IAttackRegistry.ContractState) {
address attackRegistry = safeHarborRegistry.getAttackRegistry();
if (attackRegistry == address(0)) revert InvalidAgreement();
@> return IAttackRegistry(attackRegistry).getAgreementState(agreement);
}
function claimExpired() external nonReentrant {
if (outcome == PoolStates.Outcome.UNRESOLVED) {
@> IAttackRegistry.ContractState state = _observePoolState();
if (state == IAttackRegistry.ContractState.CORRUPTED && riskWindowStart != 0) {
return;
}
if (state == IAttackRegistry.ContractState.PRODUCTION) {
outcome = PoolStates.Outcome.SURVIVED;
} else {
@> outcome = PoolStates.Outcome.EXPIRED;
}
claimsStarted = true;
}
}
Risk
Likelihood:
-
This will occur when a pool has locked scope over account X, the original agreement keeps at least one other account, the agreement owner removes X after the BattleChain commitment window, and X is registered under another agreement before being corrupted there.
-
The timing window is realistic because BattleChain requires only a 7-day minimum agreement commitment at registration, while a ConfidencePool expiry must be at least 30 days out. A pool can therefore remain live after the agreement owner is allowed to narrow the underlying agreement.
-
The required agreement update and rebinding actions are reachable through the documented BattleChain flow after the commitment window. This does require the agreement owner/sponsor to perform the narrowing or migration, but no malicious token behavior, registry compromise, reentrancy, or non-standard EVM behavior is required.
Impact:
-
The pool cannot be settled as CORRUPTED for an account it still publicly commits to cover. A moderator call to flagOutcome(CORRUPTED, ...) reverts because the original agreement is not corrupted.
-
At expiry, the pool can resolve as EXPIRED, returning principal plus bonus to stakers instead of routing the pool through bad-faith recovery or a good-faith whitehat bounty. This breaks the locked-scope guarantee and can misallocate the entire pool.
Proof of Concept
pragma solidity 0.8.26;
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
contract PerAgreementAttackRegistry {
mapping(address agreement => IAttackRegistry.ContractState state) internal _state;
mapping(address account => address agreement) internal _bindingAgreement;
function setAgreementState(address agreement, IAttackRegistry.ContractState state) external {
_state[agreement] = state;
}
function setBindingAgreement(address account, address agreement) external {
_bindingAgreement[account] = agreement;
}
function getAgreementState(address agreement) external view returns (IAttackRegistry.ContractState) {
return _state[agreement];
}
function getAgreementForContract(address account) external view returns (address) {
return _bindingAgreement[account];
}
}
contract BindingAgreementDriftTest is BaseConfidencePoolTest {
address internal constant REMAINING_AGREEMENT_ACCOUNT = address(0xBEEF);
function testLockedScopeCorruptionUnderNewBindingAgreementCannotSettleCorrupted() external {
PerAgreementAttackRegistry registry = new PerAgreementAttackRegistry();
safeHarborRegistry.setAttackRegistry(address(registry));
agreementContract.setContractInScope(REMAINING_AGREEMENT_ACCOUNT, true);
uint256 principal = 100 * ONE;
uint256 bonus = 50 * ONE;
_stake(alice, principal);
_contributeBonus(carol, bonus);
registry.setBindingAgreement(DEFAULT_SCOPE_ACCOUNT, agreement);
registry.setAgreementState(agreement, IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
assertTrue(pool.scopeLocked());
assertTrue(pool.isAccountInScope(DEFAULT_SCOPE_ACCOUNT));
vm.warp(block.timestamp + 8 days);
agreementContract.setContractInScope(DEFAULT_SCOPE_ACCOUNT, false);
assertTrue(agreementContract.isContractInScope(REMAINING_AGREEMENT_ACCOUNT));
MockAgreement agreementB = new MockAgreement(address(this));
agreementB.setContractInScope(DEFAULT_SCOPE_ACCOUNT, true);
registry.setBindingAgreement(DEFAULT_SCOPE_ACCOUNT, address(agreementB));
registry.setAgreementState(address(agreementB), IAttackRegistry.ContractState.CORRUPTED);
assertEq(registry.getAgreementForContract(DEFAULT_SCOPE_ACCOUNT), address(agreementB));
assertTrue(pool.isAccountInScope(DEFAULT_SCOPE_ACCOUNT), "pool scope remains locked");
vm.prank(moderator);
vm.expectRevert(IConfidencePool.InvalidOutcome.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
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, principal + bonus);
assertEq(token.balanceOf(recovery), 0);
}
}
Recommended Mitigation
@@
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
IAttackRegistry.ContractState state = _observePoolState();
+ bool scopedBindingCorrupted = _hasCorruptedScopedBinding();
@@
} else if (newOutcome == PoolStates.Outcome.CORRUPTED) {
if (!goodFaith_) {
if (attacker_ != address(0)) revert InvalidGoodFaithParams();
} else {
if (attacker_ == address(0)) revert InvalidGoodFaithParams();
}
- if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
+ if (state != IAttackRegistry.ContractState.CORRUPTED && !scopedBindingCorrupted) {
+ revert InvalidOutcome();
+ }
} else {
revert InvalidOutcome();
}
@@
function _getAgreementState() internal view returns (IAttackRegistry.ContractState) {
address attackRegistry = safeHarborRegistry.getAttackRegistry();
if (attackRegistry == address(0)) revert InvalidAgreement();
return IAttackRegistry(attackRegistry).getAgreementState(agreement);
}
+
+ function _hasCorruptedScopedBinding() internal view returns (bool) {
+ if (!scopeLocked) return false;
+
+ address attackRegistry = safeHarborRegistry.getAttackRegistry();
+ if (attackRegistry == address(0)) revert InvalidAgreement();
+
+ uint256 length = _scopeAccounts.length;
+ for (uint256 i; i < length; ++i) {
+ address binding = IAttackRegistry(attackRegistry).getAgreementForContract(_scopeAccounts[i]);
+ if (binding == address(0) || binding == agreement) continue;
+ if (IAttackRegistry(attackRegistry).getAgreementState(binding) == IAttackRegistry.ContractState.CORRUPTED) {
+ return true;
+ }
+ }
+
+ return false;
+ }
The core requirement is that moderator-driven corrupted settlement must not rely only on the original immutable agreement once the pool has made an account-level scope commitment. The moderator can remain the off-chain judge for whether the corrupted binding actually corresponds to the pool's locked scope, but the on-chain gate must expose a path for that judgement to be recorded. Permissionless auto-resolution should be updated only with an explicit policy decision, because agreement-level CORRUPTED is still scope-blind without moderator judgement.
