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

A permissionless race at the expiry boundary can permanently bury a confirmed CORRUPTED breach

Author Revealed upon completion

Root + Impact

Description

  • Normal behavior: an outcome is either judged by the moderator via flagOutcome, or mechanically resolved by the permissionless claimExpired() backstop — after either, claimsStarted finalizes it against re-flagging.

  • The issue: claimExpired()'s mechanical branch reads registry state live at call time and latches claimsStarted in the EXPIRED/SURVIVED branches too, not just the auto-CORRUPTED one. Whoever calls it first, at the exact moment expiry is reached, permanently decides the outcome — even if a CORRUPTED confirmation is already in flight and lands moments later.

if (state == IAttackRegistry.ContractState.CORRUPTED && riskWindowStart != 0) {
// only reachable once the registry ALREADY reads CORRUPTED
}
if (state == IAttackRegistry.ContractState.PRODUCTION) {
// SURVIVED
} else {
@> outcome = PoolStates.Outcome.EXPIRED; // reached whenever state isn't YET CORRUPTED/PRODUCTION
@> claimsStarted = true; // latched immediately — forecloses flagOutcome forever
}

Note: docs/DESIGN.md §5 already discusses a race on this function, but scopes it explicitly to riskWindowStart == 0 (no risk ever observed) and treats it as accepted. This finding is the broader, undocumented case — riskWindowStart != 0, a real attack window was observed and sealed on-chain — which the existing test suite (ClaimExpiredRegistryGate.t.sol) does not cover.

Risk

Likelihood:

  • claimExpired() carries no access control and reads registry state live at call time, so whichever caller — bot, bystander, or the sponsor — lands first at the exact expiry timestamp single-handedly decides the outcome for every other participant.

  • A real attack confirmation always has some on-chain latency between being decided upstream and being written to the registry, so there is a narrow but real window right at the boundary where the true state and the on-chain-visible state diverge.

Impact:

  • The moderator's flagOutcome(CORRUPTED, ...) becomes permanently unusable the instant the race is lost, no matter how strong the evidence of an in-scope breach is.

  • Every staker recovers full principal + bonus as though the pool simply survived, while an honest whitehat who found and reported a real vulnerability never receives their bounty.

  • recoveryAddress (the pool's insurance/treasury backstop) receives nothing, even though the underlying agreement was genuinely corrupted in-scope.

Proof of Concept

The test reproduces the race in four steps:

  1. Alice stakes 100, Carol contributes 20 bonus. _passThroughUnderAttack() seals riskWindowStart — confirming this is not the already-accepted riskWindowStart == 0 case.

  2. Time is warped to pool.expiry(). The registry still reads UNDER_ATTACK — the CORRUPTED confirmation hasn't landed yet.

  3. dave, who holds zero stake, calls claimExpired(). Outcome locks EXPIRED; claimsStarted latches.

  4. The registry updates to CORRUPTED a moment later. The moderator's flagOutcome(CORRUPTED, ...) reverts OutcomeAlreadySet. Alice claims her full 120 tokens back, as if nothing happened.

A second control test (testCorruptedConfirmationLandingFirstResolvesCorrectly) reproduces identical facts but flips the order of just two calls — CORRUPTED confirmation lands before anyone calls claimExpired() — and the pool resolves correctly, sweeping the full 120 tokens to recovery. The only variable that changes between the two tests is transaction order.

// test/unit/ConfidencePool.claimExpiredRace.t.sol
function testClaimExpiredRaceLocksExpiredBeforeCorruptedConfirmationLands() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 20 * ONE);
_passThroughUnderAttack();
assertGt(pool.riskWindowStart(), 0); // NOT the riskWindowStart==0 accepted case
vm.warp(pool.expiry());
// registry still UNDER_ATTACK: confirmation hasn't landed yet
@> vm.prank(dave); // dave holds zero stake — a pure opportunist
@> pool.claimExpired(); // locks EXPIRED, latches claimsStarted
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
vm.expectRevert(IConfidencePool.OutcomeAlreadySet.selector);
@> pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker); // too late
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimExpired();
assertEq(token.balanceOf(alice) - aliceBefore, 120 * ONE); // full refund, real breach ignored
}

Verified locally: forge test --match-contract ConfidencePoolClaimExpiredRaceTest → 2 passed.

Recommended Mitigation

The core fix is to stop finalizing EXPIRED immediately whenever a real risk window was observed and the state isn't yet decidable — instead park it behind a short, fixed settlement buffer so an in-flight CORRUPTED confirmation has a real chance to land first.

+ uint256 public constant EXPIRED_SETTLEMENT_BUFFER = 1 hours;
+ uint32 public pendingExpiredAt;
// inside claimExpired()'s outcome == UNRESOLVED branch, after the
// CORRUPTED and PRODUCTION checks:
- } else {
- outcome = PoolStates.Outcome.EXPIRED;
- outcomeFlaggedAt = expiry;
- claimsStarted = true;
- emit OutcomeFlagged(address(0), PoolStates.Outcome.EXPIRED, false, address(0));
- }
+ } else if (riskWindowStart == 0) {
+ // No risk ever observed — matches the already-accepted §5 behavior.
+ outcome = PoolStates.Outcome.EXPIRED;
+ outcomeFlaggedAt = expiry;
+ claimsStarted = true;
+ emit OutcomeFlagged(address(0), PoolStates.Outcome.EXPIRED, false, address(0));
+ } else if (pendingExpiredAt == 0) {
+ pendingExpiredAt = uint32(block.timestamp);
+ revert ExpiredPendingSettlement();
+ } else if (block.timestamp < pendingExpiredAt + EXPIRED_SETTLEMENT_BUFFER) {
+ revert ExpiredPendingSettlement();
+ } else {
+ outcome = PoolStates.Outcome.EXPIRED;
+ outcomeFlaggedAt = expiry;
+ claimsStarted = true;
+ emit OutcomeFlagged(address(0), PoolStates.Outcome.EXPIRED, false, address(0));
+ }

Support

FAQs

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

Give us feedback!