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

[H]# [H]Incorrect Player Count in Distribution Calculation Causes Unclaimed Tokens to Be Stuck in Contract

leanlabiano

[H]Incorrect Player Count in Distribution Calculation Causes Unclaimed Tokens to Be Stuck in Contract

Description:

In the Pot contract, executing the closePot function when not all players (i_players) have claimed their rewards results in a scenario where the remaining tokens become stuck in the contract and cannot be retrieved by the last players claim.
Pot::closePot calculated claimantCut with (remainingRewards - managerCut) / i_players.length and then do the distribution with for (uint256 i = 0; i < claimants.length; i++) but claimants.length != i_players.length

Impact:

After Pot::closePot It is possible to run Pot::claimCut but the last user wil be not get any reward, then their tokens are stucked in the contract, for ever!

Proof of concept:

add following test in TestMyCut.t.sol:

function testUnclaimedRewardDistributionFailed()
public
mintAndApproveTokens
{
uint256 conManBalanceBefore = ERC20Mock(weth).balanceOf(conMan);
console.log("conManBalanceBefore", conManBalanceBefore);
​
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(
players,
rewards,
IERC20(ERC20Mock(weth)),
totalRewards
);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
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);
assert(claimantBalanceAfter > claimantBalanceBefore);
// at this point all work like testUnclaimedRewardDistribution
​
// @dev get token balance for contest
uint256 contestBalanceAfter = ERC20Mock(weth).balanceOf(contest);
uint256 conManBalanceAfter = ERC20Mock(weth).balanceOf(conMan);
console.log("contestBalanceAfter", contestBalanceAfter);
console.log("conManBalanceAfter", conManBalanceAfter);
assert(conManBalanceAfter > 0);
​
vm.startPrank(player2);
vm.expectRevert();
// insufficient funds in the contract
Pot(contest).claimCut();
vm.stopPrank();
}

Recommended Mitigation:

  1. Chage Pot::closePot claimantCut calculation i_players.length for claimants.length

function closePot() external onlyOwner {
... code
- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
+ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length
}

Tools Used

  • Manual Review

  • Foundry test

Updates

Lead Judging Commences

equious Lead Judge about 1 year 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.