MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: low
Invalid

closePot() is not idempotent — remainingRewards is never zeroed and there is no closed flag, so it can be re-run to pay the manager and claimants multiple times

sub99

closePot() never zeroes remainingRewards and has no closed flag, so the owner can re-run it and re-pay the manager cut (and every claimant) repeatedly, draining the pot's balance.

Description

Normally closePot() should run exactly once per contest: skim the manager cut, distribute the remainder to claimants, and then be finished.

The issue: closePot() derives managerCut and claimantCut from remainingRewards but never zeroes it, and there is no closed guard. Both entry gates — the 90-day timestamp check and remainingRewards > 0 — stay true after a successful close, so the owner can call closeContest() again and the entire payout re-executes.

// src/Pot.sol
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) revert ...;
if (remainingRewards > 0) {
@> uint256 managerCut = remainingRewards / managerCutPercent; // remainingRewards never decremented
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (...) { _transferReward(claimants[i], claimantCut); }
}
@> // no `remainingRewards = 0`, no `closed` flag -> both gates still pass on a second call
}

Risk

Likelihood:

Requires the owner (a trusted role) to call closeContest() more than once — e.g. an accidental double-submit, a retry after a transaction that appeared to fail, or buggy keeper/automation.

There is no attacker path; the trigger is bounded to the owner role. This is why the finding is Medium rather than High.

Impact:

Each extra close re-pays the manager cut and re-pays every claimant from whatever token balance the pot still holds, over-distributing the contest's funds.

Repeated closes drain the pot's remaining balance (including any remainder stranded by the denominator bug) until a transfer reverts — a real misallocation/loss of the contest's funds.

Proof of Concept

This Foundry test passes against the in-scope code. Ten players are owed 100 (total 1000); only p1 claims (remaining 900); the owner closes twice; the manager is paid the cut both times:

function test_closePot_not_idempotent_double_manager_cut() public {
// owner funds a 1000 Pot over 10 players; only p1 claims -> remaining 900
vm.startPrank(owner);
cm.closeContest(pot); // managerCut 90 -> ContestManager
cm.closeContest(pot); // remainingRewards still 900 -> managerCut 90 AGAIN
vm.stopPrank();
assertEq(weth.balanceOf(address(cm)), 180); // 2 x 90: the manager cut is paid twice
}

Recommended Mitigation

Zero the accounting at the end of closePot() (or add a one-shot closed flag) so the gates cannot pass a second time:

function closePot() external onlyOwner {
...
+ remainingRewards = 0; // or: require(!closed, "closed"); closed = true;
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!