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

`Pot::closePot` never updates state, so it can be called again and again, paying the manager cut and the claimant bonus from the same stale `remainingRewards` each time

summus

Root + Impact

Description

  • closePot should run once: distribute the leftover pool after 90 days and leave the Pot closed.

  • The function has no "closed" flag and does not reset remainingRewards after distributing. Every later call recomputes managerCut and claimantCut from the same value and pays them again for as long as the Pot's balance allows. getRemainingRewards() keeps reporting a pool that has already been paid out.

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
@> if (remainingRewards > 0) { // still true on every later call
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);
}
@> } // remainingRewards never updated, no closed flag
}

Risk

Likelihood:

  • When the admin calls closeContest a second time for the same Pot, for example as a retry after an RPC timeout or through an off-chain script that closes all expired contests on a schedule. Nothing on-chain says the Pot is already closed, and the second call succeeds.

Impact:

  • The manager receives the 10% cut several times, which is more than the protocol defines. Combined with H-02, every extra cut is also locked in ContestManager.

  • Players whose playersToRewards is still non-zero (see M-01) find the Pot drained. getRemainingRewards() / getContestRemainingRewards() report wrong data after closing.

Proof of Concept

There are 4 players with 25 each, and only player1 claims (remainingRewards = 75). Each close pays 7 + 17 tokens. Three calls drain the Pot from 75 to 3 while remainingRewards stays 75.

function testRepeatedClosePotDrainsPot() public mintAndApproveTokens {
address player3 = makeAddr("player3");
address player4 = makeAddr("player4");
address[] memory fourPlayers = new address[](4);
fourPlayers[0] = player1; fourPlayers[1] = player2;
fourPlayers[2] = player3; fourPlayers[3] = player4;
uint256[] memory fourRewards = new uint256[](4);
fourRewards[0] = 25; fourRewards[1] = 25; fourRewards[2] = 25; fourRewards[3] = 25;
​
vm.startPrank(user);
contest = ContestManager(conMan).createContest(fourPlayers, fourRewards, IERC20(ERC20Mock(weth)), 100);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.prank(player1);
Pot(contest).claimCut();
​
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
ContestManager(conMan).closeContest(contest);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
assertEq(weth.balanceOf(contest), 3); // 75 -> 51 -> 27 -> 3
assertEq(weth.balanceOf(player1), 25 + 17 * 3);
assertEq(weth.balanceOf(conMan), 7 * 3); // cut taken three times
assertEq(Pot(contest).getRemainingRewards(), 75); // stale
}

The test uses the setup of the existing test/TestMyCut.t.sol.

Recommended Mitigation

Add bool flag "closed" and error type "Pot__AlreadyClosed" to the Pot.sol

+ error Pot__AlreadyClosed();
+ bool private closed;
​
function closePot() external onlyOwner {
+ if (closed) revert Pot__AlreadyClosed();
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
+ closed = true;
if (remainingRewards > 0) {
...
+ remainingRewards = 0;
}
}
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!