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

Distribution issue

enchev

Summary

The distribution logic to send unclaimed rewards to users who made claims is not calculating the distribution reward properly.

Vulnerability Details

One of the resposibilities of the method Pot::closePot is to distribute remaning unclaimed rewards (after manager cut) to users who already claimed.
However, when calculating the variable claimantCut it's denominator uses the players that were registered instead of only the users who made the claim.

Impact

Rewards are miscalculated, funds remain locked in contract.

Tools Used

Manual Review, Foundry

Proof Of Concept

Follow these steps to reproduce the issue:

  • Add the following member to TestMyCut:

address player3 = makeAddr("player3");
address player4 = makeAddr("player4");
address[] players_distributionExample = [player1, player2, player3, player4];
uint256[] rewards_distributionExample = [100, 300, 300, 400];
uint256 totalRewards_distributionExample = 1100;

  • Add the following method to TestMyCut:

function test_distributionMiscalculates() public mintAndApproveTokens {
​
ERC20Mock(weth).mint(user, STARTING_USER_BALANCE);
ERC20Mock(weth).approve(user, STARTING_USER_BALANCE);
vm.startPrank(user);
contest = ContestManager(conMan).createContest(
players_distributionExample, rewards_distributionExample, IERC20(ERC20Mock(weth)), totalRewards_distributionExample
);
ContestManager(conMan).fundContest(0);
totalContests = ContestManager(conMan).getContests();
vm.stopPrank();
​
ERC20Mock(weth).approve(conMan, STARTING_USER_BALANCE);
​
​
// Claim player 1 balance
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
// Close contest - distribute rewards amount users who claimed.
vm.warp(91 days);
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
​
uint256 playerBalance = ERC20Mock(weth).balanceOf(player1);
console.log(playerBalance);
​
// It's much less than 1000.
assert(playerBalance == 100 + 900);
}

  • Run the test via the following command: forge test --mt test_distributionMiscalculates

Recommendations

  • Make the following modification:

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

  • Add addional logic which handles the case where there no claimants (this would mean division by zero).
    For example if there are no claimants - send all of the prizes to the manager.

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.