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

Pre-finality bonus sweep corrupts later CORRUPTED accounting

Author Revealed upon completion

Description

sweepUnclaimedBonus() can move funds before the outcome is final while leaving claimsStarted == false. The root cause is that a later flagOutcome() re-snapshots from the mutated live accounting, so a pre-finality sweep can erase value from a later CORRUPTED reserve or let the wrong branch become permanent after value already moved.

Details

The protocol explicitly says the moderator may re-flag a typo'd outcome or attacker before any participant locks in the wrong distribution. That correction window is supposed to remain safe only until value leaves the contract. But the SURVIVED branch is still reachable even while the registry is terminal CORRUPTED, and flagOutcome() still allows a later pre-claim re-flag to CORRUPTED.

if (outcome != PoolStates.Outcome.UNRESOLVED && claimsStarted) revert OutcomeAlreadySet();
if (newOutcome == PoolStates.Outcome.SURVIVED) {
if (state != IAttackRegistry.ContractState.PRODUCTION && state != IAttackRegistry.ContractState.CORRUPTED) {
revert InvalidOutcome();
}
}

On the no-risk-window path, sweepUnclaimedBonus() treats the entire bonus as unreserved, transfers it to recoveryAddress, decrements totalBonus, and deliberately does not set claimsStarted. This is the accounting break: live economic state changes, but the contract still presents the outcome as safely re-flaggable.

if (totalEligibleStake != 0) {
reserved = totalEligibleStake;
if (riskWindowStart != 0) {
reserved += snapshotTotalBonus - claimedBonus;
}
}
uint256 amount = freeBalance > reserved ? freeBalance - reserved : 0;
if (totalEligibleStake == 0 || riskWindowStart == 0) {
totalBonus -= amount <= totalBonus ? amount : totalBonus;
}
// Intentionally does NOT set claimsStarted.
stakeToken.safeTransfer(recoveryAddress, amount);

This is the opposite of how the protocol is described externally. README.md#L71-L79 says the moderator's correction window closes on the first claim because finality is value-movement, and protocol-readme.md#L43-L45 describes sweepUnclaimedBonus() as recovering only excess over the remaining entitlement reserve. In this branch the sweep itself is the first meaningful value movement, and it does not just recover dust: it changes totalBonus before the outcome is final.

Any later re-flag then snapshots from the already-mutated live totals. flagOutcome() copies totalEligibleStake and totalBonus straight into snapshotTotalStaked, snapshotTotalBonus, corruptedReserve, and bountyEntitlement, so the future CORRUPTED path inherits the reduced reserve instead of the pre-sweep economic obligation.

snapshotTotalStaked = totalEligibleStake;
snapshotTotalBonus = totalBonus;
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;
bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;

There are then two distinct loss branches from the same root cause.

If stakers still exist, a staker can claim principal after the sweep and only then lock finality. claimSurvived() flips claimsStarted only when the claim itself executes, so the sweep creates a window where bonus is already gone but principal is still claimable through the wrong SURVIVED branch.

uint256 payout = userEligible + bonusShare;
totalEligibleStake -= userEligible;
claimedBonus += bonusShare;
if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(msg.sender, payout);

If no stakers remain, the same bug erases the future good-faith bounty entirely: the bonus is swept away while claimsStarted == false, and a later typo correction or scope correction to good-faith CORRUPTED snapshots snapshotTotalBonus == 0, even though the docs say the entire pool is the named attacker's bounty in the normal good-faith case.

This is stronger than the already-known no-risk-window claimExpired() race in docs/DESIGN.md#L111-L127. That accepted tradeoff is about a conservative EXPIRED backstop resolving before moderator action. Here the contract actively moves funds out pre-finality, then allows later re-snapshotting or a later staker claim to make the wrong distribution permanent.

The test suite also encodes the unsafe assumption instead of checking the safety claim. test_sweepUnclaimedBonus_doesNotLockReflag explicitly asserts that a successful sweep must not flip claimsStarted, and testReflagToCorruptedAfterBonusSweepDoesNotOverstateEntitlement treats the later reduced bountyEntitlement as the expected result. Those tests prove reachability of the reserve loss; they do not prove that the resulting payout is safe.

Risk

This bug silently destroys funds that a later CORRUPTED settlement should still reserve. In the good-faith path, the sponsor-controlled recoveryAddress can receive bonus that the named whitehat attacker should have received. In the bad-faith or corrected-CORRUPTED path, a staker can recover principal that should have remained sweepable. The contract therefore routes real pool value to the wrong beneficiary and underfunds the economically correct CORRUPTED outcome.

Recommended mitigation steps

Make any value-moving sweepUnclaimedBonus() call a finality event, or forbid bonus sweeps entirely while the moderator can still re-flag into a different economic branch.

Proof of concept

I validated the two strongest branches locally with a temporary standalone Foundry test file.

Save the following as test/TempSweepReflagAccounting.t.sol:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IConfidencePool} from "src/interfaces/IConfidencePool.sol";
contract TempSweepReflagAccountingTest is BaseConfidencePoolTest {
function testZeroStakerBonusSweepErasesFutureGoodFaithBounty() external {
_contributeBonus(carol, 50 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
uint256 recoveryBefore = token.balanceOf(recovery);
pool.sweepUnclaimedBonus();
assertEq(token.balanceOf(recovery) - recoveryBefore, 50 * ONE, "bonus already left the pool");
assertFalse(pool.claimsStarted(), "reflag window still open");
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
assertEq(pool.snapshotTotalBonus(), 0, "reflag snapshots the post-sweep bonus balance");
assertEq(pool.bountyEntitlement(), 0, "attacker loses the swept bonus entirely");
}
function testNoRiskSweepThenClaimLetsPrincipalEscapeFutureCorruptedPath() external {
_stake(alice, 100 * ONE);
_contributeBonus(carol, 50 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
pool.sweepUnclaimedBonus();
assertFalse(pool.claimsStarted(), "bonus sweep still leaves the correction window open");
uint256 aliceBefore = token.balanceOf(alice);
vm.prank(alice);
pool.claimSurvived();
assertEq(token.balanceOf(alice) - aliceBefore, 100 * ONE, "staker escapes with principal");
vm.prank(moderator);
vm.expectRevert(IConfidencePool.OutcomeAlreadySet.selector);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, attacker);
}
}

Run:

forge test --match-path test/TempSweepReflagAccounting.t.sol -vv

Observed output:

Solc 0.8.26 finished in 23.27s
Compiler run successful!
Ran 2 tests for test/TempSweepReflagAccounting.t.sol:TempSweepReflagAccountingTest
[PASS] testNoRiskSweepThenClaimLetsPrincipalEscapeFutureCorruptedPath() (gas: 530607)
[PASS] testZeroStakerBonusSweepErasesFutureGoodFaithBounty() (gas: 307087)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 16.15ms (4.11ms CPU time)
Ran 1 test suite in 32.72ms (16.15ms CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)

These PoCs show the same root cause producing both major failure modes:

  • the sweep can erase bonus that a later good-faith CORRUPTED path should still reserve for the named attacker

  • the sweep can happen before finality, after which a staker can still claim principal and permanently block correction into CORRUPTED

Support

FAQs

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

Give us feedback!