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

Documented bounty-hold protection silently fails to engage when the tracked entitlement is zero

Author Revealed upon completion

Root + Impact

Description

  • For good-faith CORRUPTED, bountyEntitlement is set at flag time to snapshotTotalStaked + snapshotTotalBonus, the entire pool becomes the named attacker's bounty. DESIGN.md states the intended protection plainly: a donation landing while the bounty is unclaimed is held until the attacker claims it, a bounded, recoverable delay, not a lock. The MustClaimBountyFirst gate on claimCorrupted is what enforces that hold.

    The problem is that gate only checks bountyClaimed < bountyEntitlement. If nobody has staked and nobody has called contributeBonus by flag time, both tracked sums are zero, so bountyEntitlement is zero too. The check becomes 0 < 0, which is false, so it never fires. Any raw token balance sitting in the pool, sent by direct transfer instead of through contributeBonus, is not held at all. The named attacker's own claim also fails immediately, bountyClaimed == bountyEntitlement is 0 == 0, true on their very first attempt, before the function even looks at the pool's balance.

// Root cause in the codebase with @> marks to highlight the relevant section
corruptedReserve = newOutcome == PoolStates.Outcome.CORRUPTED ? snapshotTotalStaked + snapshotTotalBonus : 0;
@> bountyEntitlement = willBeGoodFaithCorrupted ? snapshotTotalStaked + snapshotTotalBonus : 0;
function claimAttackerBounty() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
@> if (bountyClaimed == bountyEntitlement) revert BountyAlreadyClaimed();
if (!goodFaith) revert InvalidGoodFaithParams();
if (msg.sender != attacker) revert NotAttacker();
function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
@> if (goodFaith && bountyClaimed < bountyEntitlement) revert MustClaimBountyFirst();
uint256 toSweep = stakeToken.balanceOf(address(this));
if (toSweep == 0) revert NothingToSweep();

Risk

Likelihood:

  • Two things have to line up, not one. A donor has to send a raw transfer instead of calling contributeBonus, a well documented, common mistake pattern across the industry. And the pool has to still have zero tracked stake at the exact moment the attack gets flagged, which mostly narrows this to a pool that's new or hasn't attracted stakers yet.

  • Once both conditions hold, no further sophistication is needed. Any address can call claimCorrupted immediately, no delay, no relationship to the pool required.

Impact:

  • The hold that DESIGN.md describes as the intended protection does not engage at all in this case. That's a direct gap between documented behavior and actual behavior, not just an undocumented edge case.

  • Funds go to recoveryAddress, sponsor controlled, so this isn't permanent destruction. But there's no on-chain mechanism forcing or even facilitating a fix afterward, unlike the clean re-sweep path some of the other findings in this audit have. Recovery here depends entirely on the sponsor noticing and choosing to act.

  • The named attacker's own claim reverts on the first attempt regardless of any of this, they never get a chance to interact with the balance at all.

Proof of Concept

The first test proves the actual bypass. A sponsor sends a raw transfer, nobody stakes, a real breach happens, the moderator names the real whitehat good-faith CORRUPTED. bountyEntitlement computes to zero. The whitehat's own claim reverts immediately. A completely unrelated address then sweeps the entire donation with no delay at all. The second test is a control, it proves the gate works exactly as intended in the normal case, so this isn't a general break of the ordering, just the zero entitlement edge of it.
function testRawDonationToUnstakedPoolBypassesBountyGateEntirely() external {
token.mint(sponsor, 500 * ONE);
vm.prank(sponsor);
token.transfer(address(pool), 500 * ONE);
assertEq(pool.totalEligibleStake(), 0);
assertEq(pool.totalBonus(), 0);
assertEq(token.balanceOf(address(pool)), 500 * ONE);
_enterCorrupted();
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
assertEq(pool.bountyEntitlement(), 0);
vm.prank(whitehat);
vm.expectRevert(IConfidencePool.BountyAlreadyClaimed.selector);
pool.claimAttackerBounty();
uint256 recoveryBefore = token.balanceOf(recovery);
vm.prank(randomThirdParty);
pool.claimCorrupted();
assertEq(token.balanceOf(recovery) - recoveryBefore, 500 * ONE);
assertEq(token.balanceOf(whitehat), 0);
assertEq(token.balanceOf(randomThirdParty), 0);
}
function testControl_NormalBountyEntitlementGateWorksCorrectly() external {
_stake(alice, 100 * ONE);
_enterCorrupted();
vm.prank(moderator);
pool.flagOutcome(PoolStates.Outcome.CORRUPTED, true, whitehat);
assertGt(pool.bountyEntitlement(), 0);
vm.expectRevert(IConfidencePool.MustClaimBountyFirst.selector);
pool.claimCorrupted();
vm.prank(whitehat);
pool.claimAttackerBounty();
assertEq(token.balanceOf(whitehat), 100 * ONE);
}
The last three assertions in the first test are the proof, the whitehat gets nothing, the sweeper gets nothing personally, and the entire donation is already at recoveryAddress. The control test right after it shows the same gate correctly blocking a sweep when there's a real entitlement to protect.

Recommended Mitigation

Give the zero-entitlement case the same protection window the normal case already gets from sweepUnclaimedCorrupted, rather than letting it bypass any wait at all:
function claimCorrupted() external nonReentrant {
if (outcome != PoolStates.Outcome.CORRUPTED) revert OutcomeNotSet();
if (goodFaith && bountyClaimed < bountyEntitlement) revert MustClaimBountyFirst();
+ if (goodFaith && bountyEntitlement == 0 && block.timestamp <= corruptedClaimDeadline) {
+ revert MustClaimBountyFirst();
+ }
uint256 toSweep = stakeToken.balanceOf(address(this));
if (toSweep == 0) revert NothingToSweep();
corruptedClaimDeadline is already set correctly on every good-faith flag regardless of the entitlement amount, so no new state is needed. This doesn't touch the normal case at all, the new check only fires when bountyEntitlement is exactly zero, exactly the scenario this finding is about. It doesn't give the named attacker a way to actually claim the donation, that would need a bigger change to how entitlement gets computed. What it does is remove the zero-delay bypass, so a raw donation gets the same 180 day window before forfeiture that a real, tracked bounty already gets, instead of being swept away immediately with no protection at all.

Support

FAQs

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

Give us feedback!