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

closePot() never finalizes the Pot: remainingRewards is not zeroed and there is no closed guard, letting the manager re-run distribution and drain the pool

bytethebuilder

Root + Impact

Description

Pot.closePot() is the protocol's finalization step: after the 90-day window the owner takes a 10% manager cut and distributes the remainder. But it never finalizes the Pot's state - remainingRewards is read and never set to 0, and there is no closed flag or guard preventing a second call:

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
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);
}
@> // remainingRewards is never zeroed; no closed guard
}
}

Because remainingRewards keeps its pre-close value, every later closePot() call still passes the time check and re-runs the entire distribution: the manager takes another 10% cut and claimants are paid again, draining the Pot until its token balance is insufficient and transfers revert. getRemainingRewards() also permanently reports a stale, incorrect value.

Risk

Likelihood: Low - requires the Owner/Manager (marked Trusted) to call closePot() more than once; no external actor can trigger it.

Impact: High - repeated manager cuts and re-distributions drain pot funds and corrupt accounting; getRemainingRewards() no longer reflects reality.

Proof of Concept

2 players each entitled to 500 (pool = 1000). No one claims in time, so claimants is empty and remainingRewards stays 1000. The owner closes twice and collects the 10% cut both times:

function test_PoC_closePotNotFinalized() 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();
​
// nobody claims -> claimants empty, remainingRewards stays 1000
vm.warp(91 days);
​
uint256 managerBefore = ERC20Mock(weth).balanceOf(user);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest); // close #1
ContestManager(conMan).closeContest(contest); // close #2 - not blocked
vm.stopPrank();
​
// manager took the 10% cut TWICE (100 + 100 = 200) instead of once
assertEq(ERC20Mock(weth).balanceOf(user) - managerBefore, 200);
// stale accounting: still reports the full pool as remaining
assertEq(Pot(contest).getRemainingRewards(), 1000);
}

Run forge test --mt test_PoC_closePotNotFinalized -vv; it passes, proving the manager is paid twice and remainingRewards is never updated.

Recommended Mitigation

Finalize the Pot: zero remainingRewards after distribution and/or add an explicit closed guard.

if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
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 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!