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

setting `flagOutcome` to `EXPIRED` is not possible

Author Revealed upon completion

Root + Impact

setting flagOutcome to EXPIRED is not possible

As a result, several issues can happen

Description

On the Readme, it says

3. Moderator (Protocol DAO)

RESPONSIBILITIES:

  • flags the pool outcome (SURVIVED / CORRUPTED / EXPIRED) via flagOutcome, <---@

  • may re-flag a correction before the first claim, <---@

  • names the whitehat attacker for good-faith CORRUPTED.

LIMITATIONS:

  • set by the factory at clone time and immutable per-pool,

  • correction window closes on first claim (finality is value-movement), <---@

  • 180-day grace fallback exists as defense against moderator unavailability.

So the moderator can flagOutcome to EXPIRED outcome based on readme, and also can re-flag/correction before the first claim

But on the code implementation, we can see it can not, because if we try to set the newOutcome to EXPIRED, the txn will revert

So it does not follow the doc, which is a point to consider

function flagOutcome(PoolStates.Outcome newOutcome, bool goodFaith_, address attacker_) external onlyModerator {
// Re-flag allowed pre-claim so the moderator can fix a typo'd outcome / attacker before
// any participant locks in the wrong distribution. The window closing on the FIRST claim
// (`claimsStarted`) is by design — a value-movement finality latch, not a front-runnable
// moderator privilege. See docs/DESIGN.md (re-flag window).
if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet(); // ! audit-issue-maybe-known: attacker can call claim and make typo re-flag unavailable
IAttackRegistry.ContractState state = _observePoolState();
​
if (newOutcome == PoolStates.Outcome.SURVIVED) {
if (goodFaith_ || attacker_ != address(0)) {
revert InvalidGoodFaithParams();
}
// SURVIVED accepts either terminal registry state: PRODUCTION (agreement-level
// survival) or CORRUPTED (the agreement was breached but the vulnerability was
// outside this pool's committed scope — the moderator's off-chain judgement against
// the published BattleChain account list).
if (state != IAttackRegistry.ContractState.PRODUCTION && state != IAttackRegistry.ContractState.CORRUPTED) { // ! corrupted also count as survived?
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(); // <--@ if moderator tries to set newOutcome to EXPIRED
// the txn will enter into this path and revert
}

On the ConfidencePool::claimExpired We can see that it has an auto-set logic to set the outcome to EXPIRED based on IAttackRegistry.ContractState

claimExpired is only callable when the outcome is UNRESOLVED or EXPIRED

function claimExpired() external nonReentrant {
if (block.timestamp < expiry) revert PoolNotExpired();
if (outcome != PoolStates.Outcome.UNRESOLVED && outcome != PoolStates.Outcome.EXPIRED) {
revert InvalidOutcome();
}
// ...other_codes...
​
} else {
// Reached for EVERY non-terminal state, including active-risk (UNDER_ATTACK /
// PROMOTION_REQUESTED). Intentional, NOT a missing active-risk deferral: expiring
// while still attackable means the agreement survived the term, so EXPIRED
// (principal + bonus returned) is correct. See docs/DESIGN.md (EXPIRED resolution).
outcome = PoolStates.Outcome.EXPIRED;
// EXPIRED has no terminal registry observation; `expiry` is the pool's own
// deadline and is fixed at init/lock time, so it's grief-proof as the upper bound.
outcomeFlaggedAt = expiry;
emit OutcomeFlagged(address(0), PoolStates.Outcome.EXPIRED, false, address(0));
}

Risk

One of the jobs of flagOutcome is to re-flag a correction before the first claim

So let's say,

  • A ConfidencePool::flagOutcome call happened incorrectly, which changed the outcome from UNRESOLVED to a newOutcome

    • But the correct outcome should be EXPIRED

  • Now, if the moderator wants to reflag the outcome to EXPIRED before claimsStarted, he simply can not do it

    • because flagOucome has no logic for it

    • also ConfidencePool::claimExpired which set auto EXPIRED is also not possible, as the outcome has moved from UNRESOLVED to another outcome

  • So ultimately, a wrong outcome is set, like if mistakenly an outcome is set to CORRUPTED, then it creates problem


There is another risk, though

We can see that calling the ConfidencePool::claimExpired sets the outcome to EXPIRED automatically based on IAttackRegistry.ContractState
So, as flagOutcome for EXPIRED is not possible, initially calling claimExpired can set the outcome to PRODUCTION or CORRUPTED based on IAttackRegistry.ContractState

if (state == IAttackRegistry.ContractState.CORRUPTED && riskWindowStart != 0) {
----and---
if (state == IAttackRegistry.ContractState.PRODUCTION) {

So the outcome can be a wrong state, which affects claiming stake, bonus amounts

Because this risk depends on IAttackRegistry.ContractState, so I put it for another risk example

Recommended Mitigation

There should be logic for ConfidencePool::flagOutcome to set the outcome to EXPIRED also

Support

FAQs

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

Give us feedback!