MyCut

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

`closePot()` does not reset `remainingRewards` after distribution, allowing repeated calls

Description

After closePot() distributes the manager cut and claimant surplus, the remainingRewards state variable is never set to 0. The remainingRewards > 0 guard on line 53 passes on every subsequent call, causing the function to re-execute its distribution logic against an already-drained token balance.

Vulnerability Details

// src/Pot.sol, line 49-62
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) { // @> still true on second 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);
}
// @> MISSING: remainingRewards = 0;
}
}

After the first closePot() call distributes tokens, remainingRewards still holds its original value. On a second call:

  • With standard ERC20 tokens (which revert on insufficient balance): the second i_token.transfer() reverts because the Pot's token balance is now 0 or near 0. The transaction fails harmlessly but wastes the admin's gas.

  • With non-reverting tokens (if ever used despite the scope restriction): transfer() returns false silently. The function completes without error, but no tokens move. The stale remainingRewards value persists indefinitely, allowing infinite no-op calls.

In either case, the contract's remainingRewards getter permanently reports a non-zero value even after the pot is fully distributed, which is misleading for any off-chain systems reading this state.

Risk

Likelihood:

  • Requires the trusted admin to call closeContest() twice for the same pot. This is unlikely in normal operation but possible if the admin misunderstands the contract's state or if an automated system retries a transaction.

Impact:

  • With in-scope standard ERC20 tokens, the second call simply reverts — no fund loss, just wasted gas. The main concern is the permanently stale remainingRewards value misleading off-chain integrations.

PoC

The test shows that after closing a pot, remainingRewards still returns a non-zero value, and a second close attempt reverts because the token balance is already drained.

function testExploit_DoubleClose() public {
address[] memory players = new address[](2);
players[0] = makeAddr("p1");
players[1] = makeAddr("p2");
uint256[] memory rewards = new uint256[](2);
rewards[0] = 500e18;
rewards[1] = 500e18;
vm.startPrank(admin);
address contest = conMan.createContest(players, rewards, IERC20(token), 1000e18);
conMan.fundContest(0);
vm.stopPrank();
// Player1 claims
vm.prank(players[0]);
Pot(contest).claimCut();
// First close — succeeds
vm.warp(block.timestamp + 91 days);
vm.prank(admin);
conMan.closeContest(contest);
// remainingRewards NOT reset — still shows 500
assertEq(Pot(contest).getRemainingRewards(), 500e18, "Stale remainingRewards");
// Second close — reverts because token balance is drained
vm.prank(admin);
vm.expectRevert(); // ERC20 transfer reverts on insufficient balance
conMan.closeContest(contest);
}

Recommendations

Set remainingRewards = 0 after the distribution block to make the function idempotent and ensure the state accurately reflects that the pot has been closed.

for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
+ remainingRewards = 0;
}
Updates

Lead Judging Commences

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