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

No-risk-window CORRUPTED race allows permissionless EXPIRED resolution before moderator can flag

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: when a Safe Harbor agreement's in-scope contracts are attacked, the moderator is expected to call flagOutcome(CORRUPTED, ...) before any staker claims, ensuring bad-faith breaches are correctly resolved as CORRUPTED and swept per protocol rules.

  • The specific issue: if riskWindowStart == 0 on a terminal-CORRUPTED registry state, claimExpired() falls through to the EXPIRED path instead of reverting. The first successful claimExpired() call latches claimsStarted = true, after which any subsequent flagOutcome(CORRUPTED, ...) call reverts with OutcomeAlreadySet(). A staker can permissionlessly resolve the pool as EXPIRED (refunding their own principal) in the window where a breach is in-scope but the moderator has not yet flagged it, foreclosing the moderator's ability to correctly mark it CORRUPTED afterward.

function claimExpired() external nonReentrant {
if (riskWindowStart == 0 && riskWindowEnd == 0) revert RiskWindowNotReached();
// falls through to EXPIRED when riskWindowStart == 0 on terminal-CORRUPTED registry state
// @> first successful call latches claimsStarted = true
// @> subsequent flagOutcome(CORRUPTED, ...) reverts with OutcomeAlreadySet()
}

Risk

Likelihood:

  • Occurs when the registry transitions directly from a pre-attack state to terminal CORRUPTED without the pool observing an intermediate active-risk state, and the moderator has not yet flagged at the time a staker calls claimExpired().

Impact:

  • A staker with reason to avoid CORRUPTED consequences can front-run the moderator's correct flag, resolving as EXPIRED instead.

  • Confirmed and explicitly discussed in docs/DESIGN.md ("The no-risk-window CORRUPTED race") as accepted, not a bug — auto-finalizing CORRUPTED here was considered and rejected because it reintroduces the scope-blind over-punishment issue fixed in PR #63.

Proof of Concept

The following test demonstrates the race: a staker calls claimExpired() while the registry is already in a terminal CORRUPTED state but the pool has never locally observed an active-risk state (riskWindowStart == 0). This succeeds and refunds the staker's principal via the EXPIRED path. Once claimsStarted is latched to true by this call, the moderator's subsequent attempt to correctly flag the same pool as CORRUPTED reverts, because flagOutcome will not overwrite an outcome after claims have begun.

// Registry transitions directly to CORRUPTED with riskWindowStart == 0
// (no active-risk state was ever locally observed by the pool)
vm.prank(anyStaker);
pool.claimExpired();
// Succeeds: falls through to EXPIRED, refunds staker's principal, sets claimsStarted = true
vm.prank(moderator);
vm.expectRevert(OutcomeAlreadySet.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
// Reverts: moderator can no longer correctly resolve as CORRUPTED,
// even though the breach was genuinely in-scope and attacker-attributable

This confirms the race is real and reproducible: whichever party acts first (staker vs. moderator) permanently determines the pool's resolution path for this specific timing window.

Recommended Mitigation

No code change is recommended here, and this is the core point of the finding: the seemingly "obvious" fix — auto-resolving to CORRUPTED whenever the registry reports a terminal-CORRUPTED state, regardless of whether the pool observed an active-risk window — was already considered and explicitly rejected by the project team in docs/DESIGN.md. That alternative was previously implemented and then reverted in PR #63, because it reintroduces a worse bug: gating principal resolution on a local pool observation lets in-scope, genuinely-breached stakers escape via the EXPIRED path whenever the registry transitions state faster than the pool observes it, which is a strictly worse and less-fair outcome than the current race.

In other words, the current code is already the mitigation for a previously-worse version of this same tradeoff. We are not proposing an alternative implementation, since we agree with the project team's own documented reasoning that no available on-chain resolution rule is strictly better across both failure modes (staker front-running vs. scope-blind over-punishment). This finding is submitted to formally document, via independent audit confirmation, that:

  1. The race exists exactly as designed and described.

  2. The current code correctly implements the documented, intentional tradeoff.

  3. No regression to the pre-PR-63 behavior is present.

- (no code change; documented and independently confirmed as intentional)

Support

FAQs

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

Give us feedback!