MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`closePot` divides the unclaimed remainder by *all* players instead of the timely claimants — claimants are underpaid and most of the remainder is stranded forever

buggypunpun

Root + Impact

Description

Normal behavior: Per the protocol, once the 90-day claim period elapses the manager "takes a cut of the remaining pool and the remainder is distributed equally to those who claimed in time." So for a remaining pool R, the manager should receive R / 10 and the timely claimants (only claimants) should split the other 9·R/10 equally.

The issue: closePot computes the claimant share as (remainingRewards - managerCut) / i_players.length — dividing by the total number of registered players, not by the number of players who actually claimed in time (claimants.length). Every player who never claimed keeps "reserving" a slice of the remainder that no one can ever collect:

  • each timely claimant receives remainder / totalPlayers instead of remainder / claimants,

  • the undistributed portion (the shares of the players who forfeited) stays in the Pot forever — there is no withdrawal, no sweep, and closePot redistributions are capped by the pot balance,

  • if no one claimed in time, the 90% remainder beyond the manager's cut is wholly unreachable (nobody is in claimants), so the whole pool is effectively locked except the manager's 10% per call.

Root cause in src/Pot.sol:

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);
​
// @> divisor is i_players.length (ALL players), but the share is only paid to
// @> claimants — the forfeited shares of non-claimants are never distributed
// @> and remain locked in the Pot forever.
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

Risk

Likelihood:

  • Reason 1 — The underpayment and stranding occur whenever at least one player does not claim within 90 days — the exact scenario the remainder mechanism exists for; partial take-up is the norm, not the exception.

  • Reason 2 — The split runs on every closePot with remainingRewards > 0; the wrong divisor is unconditional.

Impact:

  • Impact 1 — Timely claimants receive a fraction of the bonus they are owed (their share shrinks with every non-claimant), a direct loss of entitled funds.

  • Impact 2 — The forfeited remainder (up to 90% of the leftover pool) is permanently locked in the Pot with no recovery path — tokens are stuck in the contract forever.

Proof of Concept

Foundry test (kept in poc/MyCutPoC.t.sol; drop into test/ and run) — passes, proving the underpayment and the stranded balance:

function testPoC_UnclaimedSplitUnderpaysClaimant() public {
address[] memory players = new address[](2); // player1, player2, rewards 500/500
...
vm.prank(owner);
address contest = manager.createContest(players, rewards, IERC20(address(token)), 1000);
vm.prank(owner);
manager.fundContest(0);
​
vm.prank(player1);
Pot(contest).claimCut(); // player1 claims 500 in time; player2 never claims
assertEq(token.balanceOf(player1), 500);
​
vm.warp(91 days);
vm.prank(owner);
manager.closeContest(contest);
​
// Intended (brief): player2 forfeits 500 -> manager cut 50, player1 gets 450.
// Actual: claimantCut = (500-50)/2 = 225 -> player1 receives only 225 extra.
assertEq(token.balanceOf(player1), 500 + 225, "player1 underpaid vs 450 expected");
// 225 tokens are permanently stranded in the pot (no withdrawal path).
assertEq(token.balanceOf(contest), 225, "leftover stranded in Pot");
}

Console output of the run:

$ forge test --match-contract MyCutPoC --match-test testPoC_UnclaimedSplitUnderpaysClaimant -vv
[PASS] testPoC_UnclaimedSplitUnderpaysClaimant() (gas: 1048257)
Suite result: ok. 1 passed; 0 failed; 0 skipped

Recommended Mitigation

Divide by the number of claimants who actually claimed in time, and let the pot drain fully:

- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
+ require(claimants.length > 0, Pot__NoClaimants());
+ uint256 claimantCut = (remainingRewards - managerCut) / 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
Validated
Assigned finding tags:

[H-02] Incorrect logic in `Pot::closePot` leads to unfair distribution to `claimants`, potentially locking the funds with no way to take that out

## Description in `closePot` function while calclulating the shares for claimaint cut, `i_players.length` is used, instead of `claimants.length`, causing low amount being distributed to claimants. ## Vulnerability Details [2024-08-MyCut/src/Pot.sol at main · Cyfrin/2024-08-MyCut (github.com)](https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/Pot.sol#L57) `Pot::closePot` function is meant to be called once contest passed 90 days, it sends the owner cut to owner and rest is splitted among the users who claimed b/w 90 days period. However, current implementation is wrong.&#x20; It uses total users (i_players.length) instead of the users (claimants.length) who claimed during the duration. This creates an unfair distribution to the participants and some of the funds could be locked in the contract. In worst case scenerio, it could be 90% if nobody has claimed from the protocol during the 90 days duration. ## POC In existing test suite, add following test: ```solidity function testUnfairDistributionInClosePot() public mintAndApproveTokens { // Setup address[] memory testPlayers = new address[](3); testPlayers[0] = makeAddr("player1"); testPlayers[1] = makeAddr("player2"); testPlayers[2] = makeAddr("player3"); uint256[] memory testRewards = new uint256[](3); testRewards[0] = 400; testRewards[1] = 300; testRewards[2] = 300; uint256 testTotalRewards = 1000; // Create and fund the contest vm.startPrank(user); address testContest = ContestManager(conMan).createContest( testPlayers, testRewards, IERC20(ERC20Mock(weth)), testTotalRewards ); ContestManager(conMan).fundContest(0); vm.stopPrank(); // Only player1 claims their reward vm.prank(testPlayers[0]); Pot(testContest).claimCut(); // Fast forward 91 days vm.warp(block.timestamp + 91 days); // Record balances before closing the pot uint256 player1BalanceBefore = ERC20Mock(weth).balanceOf( testPlayers[0] ); // Close the contest vm.prank(user); ContestManager(conMan).closeContest(testContest); // Check balances after closing the pot uint256 player1BalanceAfter = ERC20Mock(weth).balanceOf(testPlayers[0]); // Calculate expected distributions uint256 remainingRewards = 600; // 300 + 300 unclaimed rewards uint256 ownerCut = remainingRewards / 10; // 10% of remaining rewards uint256 distributionPerPlayer = (remainingRewards - ownerCut) / 1; // as only 1 user claimed uint256 fundStucked = ERC20Mock(weth).balanceOf(address(testContest)); // actual results console.log("expected reward:", distributionPerPlayer); console.log( "actual reward:", player1BalanceAfter - player1BalanceBefore ); console.log("Fund stucked:", fundStucked); } ``` then run `forge test --mt testUnfairDistributionInClosePot -vv` in the terminal and it will show following output: ```js [⠊] Compiling... [⠒] Compiling 1 files with Solc 0.8.20 [⠘] Solc 0.8.20 finished in 1.63s Compiler run successful! Ran 1 test for test/TestMyCut.t.sol:TestMyCut [PASS] testUnfairDistributionInClosePot() (gas: 905951) Logs: User Address: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Contest Manager Address 1: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 Minting tokens to: 0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D Approved tokens to: 0x7BD1119CEC127eeCDBa5DCA7d1Bd59986f6d7353 expected reward: 540 actual reward: 180 Fund stucked: 360 Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.58ms (506.33µs CPU time) ``` ## Impact Loss of funds, Unfair distribution b/w users ## Recommendations Fix the functions as shown below: ```diff 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; + uint256 totalClaimants = claimants.length; + if(totalClaimant == 0){ + _transferReward(msg.sender, remainingRewards - managerCut); + } else { + uint256 claimantCut = (remainingRewards - managerCut) / claimants.length; for (uint256 i = 0; i < claimants.length; i++) { _transferReward(claimants[i], claimantCut); } } + } } ```

Support

FAQs

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

Give us feedback!