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

Division by Total Players Instead of Claimants in closePot() — Tokens Permanently Locked

kivanov

Division by Total Players Instead of Claimants in closePot() — Tokens Permanently Locked

Description

When a contest's 90-day claim period expires, the owner calls closePot() to redistribute remaining unclaimed rewards among players who did claim. The function should divide the remaining pool equally among claimants only, ensuring all redistributable tokens are actually distributed.

However, closePot() divides the remaining pool by i_players.length (all registered players) instead of claimants.length (players who actually called claimCut()). This means each claimant receives far less than they should, and the undistributed difference remains permanently locked in the Pot contract, which has no sweep or withdrawal function.

// Pot.sol L49-61
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; // BUG: divides by ALL players
@> for (uint256 i = 0; i < claimants.length; i++) { // Only iterates CLAIMANTS
_transferReward(claimants[i], claimantCut);
}
}
}

Risk

Likelihood: High

  • This occurs on every single closePot() call where at least one player has not claimed. Since the protocol is designed for scenarios where not all players claim, this is the expected normal operating path.

  • There are no admin controls or alternative code paths to avoid this — it is the only redistribution mechanism.

Impact: High

  • Tokens are permanently lost proportional to the ratio of non-claimants. With 10 players and only 1 claimant, 90% of the redistributable pool is locked forever.

  • The Pot contract has no withdrawal, sweep, or self-destruct function — locked tokens are irrecoverable.

Severity: High

Proof of Concept

Consider a contest with 10 players and 1000 USDC total rewards. After 90 days, 7 players claimed and 3 did not, leaving 300 USDC remaining.

The owner calls closePot(). The manager cut is 30 USDC (300 / 10). The redistributable amount is 270 USDC. Under correct behavior, each of the 7 claimants should receive 270 / 7 ≈ 38.57 USDC. Instead, the division uses i_players.length = 10, giving each claimant only 27 USDC. Total distributed: 27 × 7 = 189 USDC. The remaining 81 USDC (30% of the redistributable pool) is permanently locked.

function test_H01_redistribution_underpay() public {
// Setup: 10 players, 1000 USDC total. 7 claim during 90 days.
address[] memory players = new address[](10);
uint256[] memory rewards = new uint256[](10);
for (uint i = 0; i < 10; i++) {
players[i] = address(uint160(i + 1));
rewards[i] = 100;
}
// Deploy and fund
Pot pot = new Pot(players, rewards, token, 1000);
token.transfer(address(pot), 1000);
// 7 players claim
for (uint i = 0; i < 7; i++) {
vm.prank(players[i]);
pot.claimCut();
}
// Warp past 90 days and close
vm.warp(block.timestamp + 91 days);
pot.closePot();
// Assert: tokens are locked
uint256 locked = token.balanceOf(address(pot));
assertGt(locked, 0, "Tokens permanently locked due to wrong denominator");
// locked = 81 USDC (30% of redistributable pool)
}

Recommended Mitigation

The fix divides by the number of actual claimants instead of total registered players. This ensures the entire redistributable pool is distributed, with no tokens left behind in the contract.

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

Lead Judging Commences

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