MyCut

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

closePot() divides claimant bonus by i_players.length instead of claimants.length, permanently locking unclaimed funds

Root + Impact

Description

  • Pot::closePot() is supposed to distribute the remaining reward pool proportionally among players who actually claimed before the deadline. Each claimant should receive an equal share of the post-manager-cut balance.

  • The divisor used to compute each claimant's share is i_players.length (total registered players) instead of claimants.length (players who called claimCut()). The loop pays out claimants.length shares but sizes each share as 1 / i_players.length of the distributable amount. Every unclaimed slot produces one share-worth of tokens that is never transferred and cannot be recovered.

// @> wrong divisor — i_players.length includes non-claimants
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
// @> loop runs claimants.length times, but cut is sized for i_players.length
_transferReward(claimants[i], claimantCut);
}

Risk

Likelihood:

  • Every time closePot runs and at least one registered player did not call claimCut before the deadline, the wrong divisor activates — which is the expected case in any real contest.

  • The protocol has no mechanism to force all players to claim, so claimants.length < i_players.length is the steady-state outcome for every contest.

Impact:

  • The fraction (i_players.length - claimants.length) / i_players.length of the distributable balance is permanently trapped in the Pot with no withdrawal path.

Proof of Concept

Deploy a pot with 100 players, fund it, have 3 players claim, then close after 90 days. The contract retains the shares that should have gone to the other 97 slots.

function testWrongDivisorLocksTokens() public mintAndApproveTokens {
uint256 numPlayers = 100;
uint256 rewardEach = 1e18;
uint256 total = numPlayers * rewardEach;
address[] memory bigPlayers = new address[](numPlayers);
uint256[] memory bigRewards = new uint256[](numPlayers);
for (uint256 i = 0; i < numPlayers; i++) {
bigPlayers[i] = address(uint160(i + 1));
bigRewards[i] = rewardEach;
}
vm.startPrank(user);
weth.mint(user, total);
weth.approve(conMan, total);
address pot = ContestManager(conMan).createContest(
bigPlayers, bigRewards, IERC20(weth), total
);
ContestManager(conMan).fundContest(
ContestManager(conMan).getContests().length - 1
);
vm.stopPrank();
// Only 3 of 100 players claim
for (uint256 i = 0; i < 3; i++) {
vm.prank(bigPlayers[i]);
Pot(pot).claimCut();
}
vm.warp(block.timestamp + 91 days);
vm.prank(user);
ContestManager(conMan).closeContest(pot);
// remainingRewards after 3 claims = 97e18
// managerCut = 97e18 / 10 = 9.7e18
// distributable = 87.3e18
// claimantCut per iteration = 87.3e18 / 100 = 0.873e18
// 3 claimants receive 3 * 0.873e18 = 2.619e18
// locked: 87.3e18 - 2.619e18 = 84.681e18
assertGt(weth.balanceOf(pot), 0, "tokens permanently locked in pot");
}

weth.balanceOf(pot) > 0 after close proves that tokens remain trapped with no recovery path.

Recommended Mitigation

Replace i_players.length with claimants.length as the divisor so the entire distributable balance is paid out to actual claimants. Add a zero-claimants guard to avoid division by zero.

- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
- for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
- }
+ if (claimants.length == 0) {
+ i_token.transfer(i_manager, remainingRewards - managerCut);
+ } else {
+ 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 13 days 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!