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

`Pot.sol::closePot` Does not correctly calculate `claimantCut`

coffee

Summary

Pot.sol::closePot Does not correctly calculate claimantCut leaving a balance in the Pot contract lost forever. It is also subject to precision loss and an unequal distribution to the claiming players.

Vulnerability Details

In the closePotfunction, when claimantCut is calculated, it uses i_players to determine how many players will be receiving a cut of the remaining unclaimed reward.

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

However, this is incorrect, because not every player will be recieving the remaining reward. it should use claimants.lengthbecause it will only be distributing to the claimants.

for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}

The Calculation of this amount should also be checked for precision loss, in the case where the remaining amount is not evenly divisible by the number of players (or claimants) there will be an unequal amount distirbuted to each player/claimant.

Impact

The test below shows that there is a leftover amount of reward stuck in the Pot contract

function testUnclaimedRewardDistribution() public mintAndApproveTokens {
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);
​
uint256 balanceAfterContestClose = ERC20Mock(weth).balanceOf(contest);
​
assert(claimantBalanceAfter > claimantBalanceBefore);
​
assert(balanceAfterContestClose == 225);
}

Tools Used

--Foundry

Recommendations

It is recommended to change how claimantCut is calculated.

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 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}
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.