Timely claimants are underpaid because closePot() divides the remaining pool by all players instead of claimants
Root + Impact:
In Pot.closePot(), the contract intends to distribute the remaining rewards to users who already claimed during the 90-day claim period. However, the per-claimant reward is calculated using i_players.length.
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);
}
The loop only transfers to claimants, but the denominator is all original players. Therefore, when not every player claimed in time, each claimant receives less than their correct share. The undistributed difference remains inside the Pot contract with no withdrawal path.
Description:
According to the project README, authorized claimants have 90 days to claim their cut. After that period, the manager takes a cut of the remaining pool, and the rest should be distributed equally to the users who claimed in time.
The implementation partially follows this logic because closePot() loops over the claimants array. This array only contains users who successfully called claimCut().
However, the amount sent to each claimant is calculated as:
(remainingRewards - managerCut) / i_players.length
This is incorrect because i_players.length includes both users who claimed and users who did not claim. The correct denominator should be claimants.length.
This causes underpayment whenever only a subset of players claim before the pot is closed.
Risk:
Likelihood is medium because it is normal for some authorized users to miss a claim deadline.
Impact is medium because users who behaved correctly and claimed in time receive less than promised by the protocol. The remaining undistributed funds stay locked in the Pot, since there is no function that later sweeps or redistributes those funds.
Proof of Concept:
Assume the following contest setup:
Total players: 4
Initial reward per player: 100 tokens
Total rewards: 400 tokens
Only Alice claims during the 90-day claim period.
After Alice claims:
Alice receives: 100
remainingRewards = 300
claimants.length = 1
i_players.length = 4
After 90 days, the owner closes the pot.
The manager cut is:
managerCut = 300 / 10 = 30
The amount left for timely claimants should be:
300 - 30 = 270
Since Alice is the only claimant, Alice should receive all 270 tokens.
Expected result:
Alice extra reward = 270
Total distributed on close = 30 + 270 = 300
Pot balance after close = 0
Actual result:
claimantCut = 270 / i_players.length
claimantCut = 270 / 4
claimantCut = 67
Only Alice receives 67.
Actual final accounting:
Manager receives: 30
Alice receives: 67
Total distributed on close: 97
Tokens stuck in Pot: 203
So the timely claimant is underpaid by 203 tokens, and those tokens remain locked in the contract.
A Foundry-style test would look like:
function testClosePotUnderpaysClaimantsWhenSomePlayersDoNotClaim() public {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address carol = makeAddr("carol");
address dave = makeAddr("dave");
}
Lead Judging Commences
[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.  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); } } + } } ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.