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

If there are enough tokens stuck in contract one of the users can actually withdraw his TOKEN even after the contest is closed

hawksvision

Summary

It is specified in the contract logic that the players are allowed to withdraw the rewards only during the 90 days timeline and whenthe contest is up and running but it's possible for players to withdraw amount even after the contest is over as there is another logical flaw which makes some Tokens to be stuck in the contract itself.

Vulnerability Details

There is a logical flaw in calculation of the claimantcut which results in tokens being stuck in the contract in addition to this there is a possibility for users to withdraw the stuck TOKENS even after the pot is closed. Actually according to the theoritical explaination of the contract once the pot is closed there shouldn't be any remaining TOKENS in contract and user shouldn't be able to claim any rewards.

Proof of Concept

Scenario coded for POC:

  1. Four players are having rewards points each of values (500,300,150 and 50) respectively.

  2. During the timeline for claiming rewards two players collected their rewards of 500 and 300 leaving 200 TOKENS to be available in the contract.

  3. Once the manager calls the closeContest function 10% of the remaining Tokens will be transferred to the manager, and the claimantcut value becomes 45 (claimantcut =( remaning(200) - managerCut(20) )/ playersCount(4)).

  4. After transferring the 45 TOKENS to both the claiments there will be remaining 90 tokens Stuck in the contest these should have been distributed as well.

  5. Now the player4 who has 50 reward points can claim his share by calling the claimCut function as there is no logic to make the total rewards to zero after closing the contest or a condition to stop the player from withdrawing after 90 days.

function testGetTokensAfterEnd() public mintAndApproveTokens {
vm.startPrank(user);
rewards = [500, 300, 150,50];
totalRewards = 1000;
contest = ContestManager(conMan).createContest(players1, rewards, IERC20(ERC20Mock(weth)), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.startPrank(player2);
Pot(contest).claimCut();
vm.stopPrank();
​
​
//uint256 claimantBalanceBefore = ERC20Mock(weth).balanceOf(player1);
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest);
vm.stopPrank();
​
console.log("balance Stuck in Contract",ERC20Mock(weth).balanceOf(contest));
​
uint256 playerBalanceBefore = ERC20Mock(weth).balanceOf(player4);
vm.startPrank(player4);
Pot(contest).claimCut();
vm.stopPrank();
uint256 playerBalanceAfter = ERC20Mock(weth).balanceOf(player4);
​
​
vm.startPrank(player3);
vm.expectRevert();
Pot(contest).claimCut();
vm.stopPrank();
​
​
//uint256 claimantBalanceAfter = ERC20Mock(weth).balanceOf(player1);
assert(ERC20Mock(weth).balanceOf(contest)>0);
assert(playerBalanceAfter>playerBalanceBefore);
}

Tools Used

-> Manual review.

-> Foundry.

Recommendations

There are three recommandations for this contract.

  1. change claimentCut formula as follows:

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

2.Add a time check in the claimCut() function, It can be something similar to the below code.

function claimCut() public {
if (block.timestamp - i_deployedAt > 90 days) {
revert Pot__TimeElapsed();
}
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player);
_transferReward(player, reward);
}

3.Also add a logic to make the total rewards as zero in the end of closeContract function.

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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