Description:
The contract has two structurally identical "sweep whatver nobody claimed" functions. Simply one for the SURVIVED/EXPIRED bonus leftovers and one for the CORRUPTED good-faith leftovers.
As seen from the 2 functions:
Both are permissionless.
Both gate on a deadline/outcome check.
Both transfer residual funds to recoveryaddress.
Their treatment towards the claimStarted bool, the finality latch that permanently close the moderator's flagOutcome re-flag correction window diverges:
The sweepUnclaimedBonus function:
function sweepUnclaimedBonus() external nonReentrant {
...
stakeToken.safeTransfer(recoveryAddress, amount);
}
The sweepUnclaimedCorrupted function:
function sweepUnclaimedCorrupted() external nonReentrant {
...
if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(recoveryAddress, amount);
}
As observed, the claimStarted is documented in the docs/design.md as a value-movement finality latch (closing the moderator's window specifically because " a claim is the distribution-locking event"). The bonus side comment firmly defends why a sweep should never trip this latch: a trivial donation on the other hand could otherwise let anyone grief the moderator's correction window through a non-claim entrypoint.
The sweepUnclaimedCorrupted function is permissionless(triggered by anyone). Moving a possibly donation-inflated balanceOf amount but received the opposite treatement with no comment explaining the divergence.
Impact:
From my lens, a permissionless sweep, not a claim can close the moderator's correction window. If the named attacker never claims within the 180 day window, sweepUnclaimedCorrupted becomes the first interaction to set claimsStarted thus permanently blocking any flagOutcome correction i.e a wrong attacker address even though no participant ever actually claimed a payout. This breaks the finality guarantee the contract documents for itself: correction access should end on value movement not on cleanup.
Proof of Concept:
Kindly place this under test/SweepClaimsStartedAssymetry.t.sol and run:
forge test --mt test_SweepUnclaimedCorrupted_ClosesReflagWindow_UnlikeSweepUnclaimedBonus -vvvv
pragma solidity >=0.8.26;
import {Clones} from "@openzeppelin/contracts/proxy/Clones.sol";
import {ConfidencePool} from "src/ConfidencePool.sol";
import {PoolStates} from "src/libraries/PoolStates.sol";
import {IAttackRegistry} from "@battlechain/interface/IAttackRegistry.sol";
import {BaseConfidencePoolTest} from "test/helpers/BaseConfidencePoolTest.sol";
import {MockAttackRegistry} from "test/mocks/MockAttackRegistry.sol";
import {MockSafeHarborRegistry} from "test/mocks/MockSafeHarborRegistry.sol";
contract SweepClaimsStartedAsymmetryTest is BaseConfidencePoolTest {
function test_SweepUnclaimedCorrupted_ClosesReflagWindow_UnlikeSweepUnclaimedBonus() external {
address namedAttacker = makeAddr("namedAttacker");
_stake(alice, 100 * ONE);
_contributeBonus(carol, 20 * ONE);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.UNDER_ATTACK);
attackRegistry.setAgreementState(IAttackRegistry.ContractState.CORRUPTED);
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, namedAttacker);
assertEq(uint256(pool.outcome()), uint256(PoolStates.Outcome.CORRUPTED));
assertFalse(pool.claimsStarted(), "claimsStarted must start false");
vm.warp(pool.corruptedClaimDeadline() + 1);
pool.sweepUnclaimedCorrupted();
assertTrue(pool.claimsStarted(), "sweepUnclaimedCorrupted incorrectly latches claimsStarted");
vm.prank(moderator);
vm.expectRevert();
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, alice);
MockSafeHarborRegistry safeHarborRegistryB = new MockSafeHarborRegistry();
MockAttackRegistry attackRegistryB = new MockAttackRegistry();
safeHarborRegistryB.setAttackRegistry(address(attackRegistryB));
safeHarborRegistryB.setAgreementValid(agreement, true);
ConfidencePool implementation = new ConfidencePool();
ConfidencePool poolB = ConfidencePool(Clones.clone(address(implementation)));
address[] memory scope = new address[](1);
scope[0] = DEFAULT_SCOPE_ACCOUNT;
poolB.initialize(
agreement,
address(token),
address(safeHarborRegistryB),
moderator,
block.timestamp + 31 days,
ONE,
recovery,
address(this),
scope
);
token.mint(alice, 100 * ONE);
vm.prank(alice);
token.approve(address(poolB), 100 * ONE);
vm.prank(alice);
poolB.stake(100 * ONE);
token.mint(carol, 20 * ONE);
vm.prank(carol);
token.approve(address(poolB), 20 * ONE);
vm.prank(carol);
poolB.contributeBonus(20 * ONE);
attackRegistryB.setAgreementState(IAttackRegistry.ContractState.PRODUCTION);
vm.prank(moderator);
poolB.flagOutcome(PoolStates.Outcome.SURVIVED, false, address(0));
assertEq(uint256(poolB.outcome()), uint256(PoolStates.Outcome.SURVIVED));
assertFalse(poolB.claimsStarted(), "claimsStarted must start false on the sibling pool too");
poolB.sweepUnclaimedBonus();
assertFalse(poolB.claimsStarted(), "sweepUnclaimedBonus correctly leaves claimsStarted false");
}
}
Summary: Side A resolves CORRUPTED, the named attacker never claims and sweepUnclaimedCorrupted after the claim window latches claimsStarted and blocks a subsequent moderator re-flag whereas Side B resolves SURVIVED under the identical "nobody claimed" conditions and sweepUnclaimedBonus correctly leaves claimsStarted to false
Recommended Mitigation:
Kindly apply the same reasoning used for sweepUnclaimedBonus to sweepUnclaimedCorrupted by omitting the claimsStarted bool since genuine reliance on the outcome should come from a participant's own claim, not a permissionless residual sweep.
function sweepUnclaimedCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (!goodFaith) revert NotGoodFaithCorrupted();
if (block.timestamp <= corruptedClaimDeadline) revert ClaimWindowNotExpired();
// aderyn-fp-next-line(reentrancy-state-change)
uint256 amount = stakeToken.balanceOf(address(this));
if (amount == 0) revert NothingToSweep();
corruptedReserve = 0;
bountyClaimed = bountyEntitlement;
- if (!claimsStarted) claimsStarted = true;
stakeToken.safeTransfer(recoveryAddress, amount);
emit UnclaimedCorruptedSwept(msg.sender, recoveryAddress, amount);
}
