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

remainingRewards is never decremented in closePot(), allowing repeated payouts from the same pool

ranktang

Root + Impact

Description

  • remainingRewards is meant to track how much of the pot is still unclaimed. It is correctly decremented in claimCut() when a player claims, but it is never decremented in closePot() after the leftover is distributed.

    Because closePot() uses remainingRewards to compute both the manager cut and the claimant cut, and does not update it afterward, the variable keeps its old value. If closePot() is called again, the contract re-computes the same cuts from the same stale remainingRewards and pays them out again.

    This lets the manager and the claimants drain extra funds from the contract beyond what should be distributed, and the pot still cannot be properly closed.

if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
​
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}

There is no:

remainingRewards = 0;

or any other update to remainingRewards after distribution.

In contrast, claimCut() does update it:

remainingRewards -= reward

So the intent is clearly for remainingRewards to reflect the unclaimed balance. closePot() simply forgets to zero it out.

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Each call to closePot() pays the manager cut and the claimant cut again from the same remainingRewards.

  • The manager receives multiple cuts from the same leftover pool.

  • Claimants receive multiple payouts from the same leftover pool.

  • Funds that should stay in the contract until proper accounting are drained early.

  • The pot can never reach a clean "closed" state because the variable never moves.

  • Anyone who can trigger closePot() (the owner, since it is onlyOwner) can drain the contract beyond the intended distribution.

Even if the owner is trusted, this is still a correctness bug: the contract's internal accounting is inconsistent with its token balance, and a single accidental double call causes overpayment.

Proof of Concept

Setup:

  • players = [player1, player2]

  • rewards = [500, 500]

  • totalRewards = 1000

  • managerCutPercent = 10


  • player1 claims 500. Now remainingRewards = 500.

  • After 90 days, owner calls closePot().

    • managerCut = 500 / 10 = 50

    • claimantCut = (500 - 50) / 2 = 225

    • player1 receives 225, manager receives 50.

    • remainingRewards is still 500.

  • Owner calls closePot() again.

    • managerCut = 500 / 10 = 50 again.

    • claimantCut = 225 again.

    • player1 receives another 225, manager receives another 50.

    • remainingRewards is still 500.

function testRepeatedClosePotPaysTwice() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.warp(91 days);
​
uint256 managerBalanceBefore = ERC20Mock(weth).balanceOf(user);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
uint256 managerBalanceAfterFirst = ERC20Mock(weth).balanceOf(user);
​
ContestManager(conMan).closeContest(contest);
uint256 managerBalanceAfterSecond = ERC20Mock(weth).balanceOf(user);
vm.stopPrank();
​
// Manager was paid the same cut twice from the same remainingRewards
assertEq(managerBalanceAfterFirst - managerBalanceBefore, 50);
assertEq(managerBalanceAfterSecond - managerBalanceAfterFirst, 50);
}

Recommended Mitigation

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
+ uint256 distributable = remainingRewards - managerCut;
+
+ // Reset before external calls to prevent re-entrancy from re-paying the same pool
+ remainingRewards = 0;
+
i_token.transfer(msg.sender, managerCut);
​
- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
- for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
- }
+ if (claimants.length > 0) {
+ uint256 claimantCut = distributable / claimants.length;
+ for (uint256 i = 0; i < claimants.length; i++) {
+ _transferReward(claimants[i], claimantCut);
+ }
+ }
}
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 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!