MyCut

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

Incorrect divisor in `closePot()` uses `i_players.length` instead of `claimants.length`, causing claimants to receive less than their fair share and leaving funds permanently stuck

Risk + Impact :
closePot() divides the leftover reward pool by i_players.length instead of claimants.length, causing claimants to receive less than their promised share while the shortfall becomes permanently stuck in the contract.

Description

  • After the manager's cut is deducted, the remaining reward pool should be split equally among all claimants who successfully claimed before the Pot closed.

  • Instead, closePot() divides the leftover pool by i_players.length (the total number of original players) rather than claimants.length (the number of players who actually claimed), causing each claimant to receive less than their fair share.

uint256 claimantCut = (remainingRewards - managerCut) /
i_players.length; @> // should be claimants.length

Risk

Likelihood:

  • This occurs whenever any authorized player fails to claim within the 90-day window, which is the expected common case since 100% claim participation across all players is unrealistic.

  • The Pot's own redistribution mechanism exists specifically to handle unclaimed rewards, confirming the protocol itself expects some players not to claim.

Impact:

  • Claimants receive less than their promised equal share of the leftover pool — quantified in the PoC as 225 tokens received instead of the fair 450.

  • The shortfall (the difference between what claimants should have received and what they actually received) is permanently locked in the Pot contract, with no function anywhere to recover it.

Proof of Concept


The following Foundry test demonstrates the bug: 2 players are added to a Pot with 500 tokens each (1000 total). Only player1 claims their initial share; player2 never claims. After the 90-day window closes, player1 should receive the full leftover amount (450, after the 10% manager cut), but instead only receives 225 — with the remaining 225 tokens left stuck in the `Pot` contract.
Test output confirms: player1 receives exactly 225 instead of the fair 450, and the `Pot` contract retains a stuck balance of 225 tokens after closing.
function testUnfairRedistributionDueToWrongDivisor() 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();
// Only player1 claims — player2 never shows up
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
uint256 claimantBalanceBefore = ERC20Mock(weth).balanceOf(player1);
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
uint256 claimantBalanceAfter = ERC20Mock(weth).balanceOf(player1);
uint256 actualBonus = claimantBalanceAfter - claimantBalanceBefore;
// Correct behavior: 450 leftover / 1 real claimant = 450
uint256 fairBonus = 450;
// Bug: code divides by i_players.length (2), not claimants.length (1)
assertEq(actualBonus, 225); // buggy output
assertLt(actualBonus, fairBonus); // claimant was shortchanged
// The missing 225 tokens are permanently stuck in the Pot
uint256 potBalanceAfterClose = ERC20Mock(weth).balanceOf(contest);
assertEq(potBalanceAfterClose, 225);
}

Recommended Mitigation

Divide the leftover reward pool by claimants.length instead of i_players.length, so the full remaining amount is distributed among the players who actually claimed:

- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
+ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
Updates

Lead Judging Commences

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