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

Improper calculation of claimantCut results in tokens getting stuck in the contract

hawksvision

Summary

Improper calculation of the claimantCut results in the tokens being stuck in the contract,claimantCut which is used to distribute the unclaimed tokens between the players is calculated wrongly.

Vulnerability Details

The claimantCut variable in the closePot is used to distribute the unclaimed amount but instead of using claimants length in the calculation, the contract uses player length which in turn leads to reward tokens getting stuck in the contract itself.

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

Proof Of Concept

Steps to recreate:

  1. Create a contest with two players and give 500 rewards for each player.

  2. Fund the contest and claim the reward for one player.

  3. End the contest,now the contract will send 50 tokens from the unclaimed tokens to the manager.

  4. Remaining 450 tokens will be split into two as per the calculation and one part of the split will be send to the player1 as he is a claimant.

  5. Remaining half which equals to 225 tokens will be stuck in the contract itself.

function testCalculationOfClaimantCut() public mintAndApproveTokens {
//Creating the contest and funding
vm.startPrank(user);
rewards = [500, 500];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players1, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
//player1 claims his rewards
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.warp(91 days);
​
//end the contest and distribute the unclaimed amount
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
//contract balance should be zero but it isn't
assertEq(ERC20Mock(weth).balanceOf(contest),225);
}

Tools Used

-> Manual Review

-> Foundry

Recommendations

instead of using player length for catculating claimantCut use Claimant length like shown in the following code:

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);
// using claimant length instead of player 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 10 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.