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

The agreement commitment can expire before the pool term, allowing a locked-scope account to be detached and preventing CORRUPTED settlement

Author Revealed upon completion

initialize and setExpiry never require the agreement's commitment window to cover the pool term, letting the sponsor narrow scope mid-term and void coverage of a still-published account

Severity: Medium

Description

  • The pool prices settlement on its locked scope, reserving a CORRUPTED outcome for when an in-scope account was the breach surface. This holds only while the agreement's own scope is immutable for at least as long as the pool is live. The agreement enforces immutability only for its commitment window (getCantChangeUntil), whose registration floor is MIN_COMMITMENT = 7 days, whereas the pool term floor is _MIN_EXPIRY_LEAD = 30 days.

  • Neither initialize() nor setExpiry() requires getCantChangeUntil() >= expiry. Once the commitment window lapses, the agreement owner calls removeAccounts() — which has no attack-state gate — to remove a covered account while the agreement is still UNDER_ATTACK, deleting its registry binding. The pool keeps publishing the account as covered but reads only the single stored agreement; if that account is later corrupted (including after rebinding to another agreement), the stored agreement never reports CORRUPTED, so flagOutcome(CORRUPTED) reverts InvalidOutcome. The pool mis-settles SURVIVED / EXPIRED and returns principal and bonus.

// src/ConfidencePool.sol — initialize(): term length is checked, commitment vs. expiry is not.
if (expiry_ < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
if (!IBattleChainSafeHarborRegistry(safeHarborRegistry_).isAgreementValid(agreement_)) revert InvalidAgreement();
@> // missing: require IAgreement(agreement_).getCantChangeUntil() >= expiry_
// src/ConfidencePool.sol — all settlement reads the one stored agreement, never the account's current binding.
function _getAgreementState() internal view returns (IAttackRegistry.ContractState) {
address attackRegistry = safeHarborRegistry.getAttackRegistry();
if (attackRegistry == address(0)) revert InvalidAgreement();
@> return IAttackRegistry(attackRegistry).getAgreementState(agreement);
}
// src/ConfidencePool.sol — flagOutcome() requires the stored agreement to be CORRUPTED, so the moderator
// cannot settle CORRUPTED for a covered account breached under any other agreement.
} else if (newOutcome == PoolStates.Outcome.CORRUPTED) {
@> if (state != IAttackRegistry.ContractState.CORRUPTED) revert InvalidOutcome();
}

The remove-while-under-attack unbinding is exercised by the vendored test testUnregisterContractForExistingAgreement_HappyPath (lib/battlechain-safe-harbor-contracts/test/unit/AttackRegistryTest.t.sol).

Risk

Likelihood:

  • A pool outlives the agreement's commitment window whenever expiry exceeds the remaining getCantChangeUntil; the pool floor (30 days) exceeds the commitment floor (7 days) and the two are never checked against each other.

  • The agreement owner removes a covered account once the commitment window lapses; removeAccounts() is gated only on block.timestamp >= s_cantChangeUntil, so it executes while the agreement is still UNDER_ATTACK.

Impact:

  • Coverage of the removed account is voided while the pool keeps publishing it, so stakers and bonus contributors rely on coverage the protocol can no longer enforce.

  • Up to the entire principal and bonus is returned to stakers instead of routed to the whitehat or recoveryAddress, despite the sole published-scope account being breached during the term.

Proof of Concept

A per-agreement registry mock is used because the repo's default MockAttackRegistry holds a single global state.

// test/audit/ConfidencePoolM03.t.sol
function test_M03_RemovedAndReboundScopeAccountCannotTriggerCorrupted() external {
address x = DEFAULT_SCOPE_ACCOUNT;
PerAgreementAttackRegistryMock registry = new PerAgreementAttackRegistryMock();
safeHarborRegistry.setAttackRegistry(address(registry));
address agreementA = agreement;
registry.setAgreementState(agreementA, IAttackRegistry.ContractState.NEW_DEPLOYMENT);
​
pool.setExpiry(block.timestamp + 60 days); // pool term 60d > modeled 30d commitment
_stake(alice, 100 * ONE);
_contributeBonus(carol, 20 * ONE);
​
registry.setAgreementState(agreementA, IAttackRegistry.ContractState.UNDER_ATTACK);
pool.pokeRiskWindow();
​
vm.warp(block.timestamp + 31 days); // past commitment, pool still live
​
// removeAccounts(): X leaves agreement A; binding deleted; pool still publishes X.
agreementContract.setContractInScope(x, false);
assertTrue(pool.isAccountInScope(x));
​
// X rebound to agreement B and corrupted there; agreement A reaches PRODUCTION.
MockAgreement agreementB = new MockAgreement(address(this));
agreementB.setContractInScope(x, true);
registry.setAgreementState(address(agreementB), IAttackRegistry.ContractState.CORRUPTED);
registry.setAgreementState(agreementA, IAttackRegistry.ContractState.PRODUCTION);
​
vm.prank(moderator);
vm.expectRevert(IConfidencePool.InvalidOutcome.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
​
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
uint256 before = token.balanceOf(alice);
vm.prank(alice);
pool.claimSurvived();
assertEq(token.balanceOf(alice) - before, 120 * ONE); // paid out despite X being corrupted
}

Recommended Mitigation

Require the agreement's commitment window to cover the pool term wherever the term is set. The window can only be extended, never shortened, so the check is durable once passed.

+ error AgreementCommitmentTooShort(uint256 commitmentEnd, uint256 requiredEnd);
​
function initialize(...) external initializer {
if (expiry_ < block.timestamp + _MIN_EXPIRY_LEAD) revert ExpiryTooSoon();
+ 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();
+ uint256 commitmentEnd = IAgreement(agreement).getCantChangeUntil();
+ if (commitmentEnd < newExpiry) revert AgreementCommitmentTooShort(commitmentEnd, newExpiry);
...
}

Support

FAQs

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

Give us feedback!