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

[H1] `Pot::closePot` Incorrectly Uses `i_players.length` Instead of `claimants.length` for Reward Distribution leading tokens to stuck in `Pot` contract forever.

tusharr1411

Summary

In the Pot::closePot function, the calculation for claimantCut uses i_players.length instead of claimants.length. This results in an incorrect distribution of remaining rewards. As i_players contains more addresses than claimants, the calculated claimantCut will be too low, leaving some tokens unreleased in the contract. The correct approach should be to divide the remaining rewards among the actual number of claimants.

Impact

The use of i_players.length instead of claimants.length means that the rewards are distributed based on the total number of players, not just those who have actually claimed. This leads to:

  • Unclaimed Tokens: remianing token will remain stuck in the contract fprever if the number of players is greater than the number of claimants.

  • Inequitable Distribution: Claimants receive less than they should, as the calculation does not accurately reflect the number of individuals who are eligible to claim rewards.

Vulnerability Details

Proof of Concept:

  1. Suppose i_players contains 5 addresses and claimants contains 3 addresses.

  2. remainingRewards is 1000 tokens, and the manager cut is calculated as 100 tokens (10%).

  3. The current implementation calculates claimantCut as (1000 - 100) / 5, which results in 180 tokens per player.

  4. However, there are only 3 claimants, so each should receive (1000 - 100) / 3 = 300 tokens instead.

  5. As a result, 3 claimants receive 180 tokens each (totaling 540 tokens), leaving 460 tokens stuck in the contract.

Proof of Code (PoC):

Code

place the following in the TestMyCut.t.sol::TestMyCut

function test_ClosePotDoesNotDistributeUnclaimedRewardsCorrectly() public mintAndApproveTokens {
// Initialize the pot with 5 players, only 2 of which will claim.
players = [player1, player2, player3, player4, player5];
rewards = [100, 200, 300, 400, 500];
totalRewards = 1500;
​
// Create and fund the pot
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
// Simulate claims
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.startPrank(player2);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.warp(91 days);
​
uint256 unclaimedReward = Pot(contest).getRemainingRewards(); // 1500-100-200 = 1200
uint256 managerCut = unclaimedReward/10; //1200/10 = 120
​
uint256 beforePotClosingBalanceOfClaimant_Player1 = ERC20Mock(weth).balanceOf(player1);
​
// Close the Pot
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
uint256 afterPotClosingBalanceOfClaimant_Player1 = ERC20Mock(weth).balanceOf(player1);
​
// after the closing of Pot
uint256 expectedContestBalance = 0;
uint256 actualContestBalance = ERC20Mock(weth).balanceOf(contest);
​
uint256 expectedRewardPerClaimant = ( unclaimedReward - managerCut) / 2;// (1200-120)/2 = 540
uint256 actualRewardPerClaimant = afterPotClosingBalanceOfClaimant_Player1 - beforePotClosingBalanceOfClaimant_Player1;
​
// console.log(actualContestBalance , expectedContestBalance);
// console.log(actualRewardPerClaimant, expectedRewardPerClaimant);
​
assert(actualContestBalance > expectedContestBalance);
assert(actualRewardPerClaimant < expectedRewardPerClaimant);
​
}
​
​

Tools Used

Manual Review

Recommendations

Use claimants.length for Distribution: Replace i_players.length with claimants.length to accurately distribute rewards only among the claimants.

...
​
- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
+ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
​
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
Updates

Lead Judging Commences

equious Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Incorrect distribution in closePot()

Support

FAQs

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