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

`ContestManager::createContest` takes in a `rewards` array and a `totalRewards` parameter , but doesn't check to see whether the rewards sum up to the total rewards. If sum is less , this makes some users unable to claim their rewards

SoulReaper

Summary

ContestManager::createContest takes in a rewards array and a totalRewards parameter , but doesn't check to see whether the rewards sum up to the total rewards. If sum is less , this makes some users unable to claim their rewards

Vulnerability Details

ContestManager::createContest is used by owner to create a new contest . 2 of its parameters are :

  1. rewards - array of rewards to be distributed to players

  2. totalRewards - (should be -> ) the sum of all the rewards in the rewards array

But this function doesn't check to see whether sum of all the rewards in the rewards array is actually equal to totalRewards or not. Consider 3 cases:

  1. totalRewards > sum

    1. All users can claim

    2. Leftover rewards distributed via Pot::claimPot function

    3. But owner wouldn't wanna give away more rewards than what is specified in the rewards array . so this scenario is unwanted , even though it doesn't revert anywhere.

  2. totalRewards == sum

    1. Everything works normally

  3. totalRewards < sum

    1. Some users may face reverts while claiming since contract doesn't have as much balance as it is supposed to have

    2. If somebody doesn't claim and owner calls claimPot , this call will go through without reverts as it works on ratio calculation and not absolute values

    3. But obviously this scenario is unwanted as users weren't able to claim what they deserved.

The only case that the protocol intends to handle is case no. 2 , so we should only allow the owner to create a pot which corresponds to case 2 , i.e. totalRewards == sum

Impact

If owner doesn't input totalRewards correctly , the owner or users may lose out on funds

Proof of Concepts
I have written 4 tests to prove cases 1 and 3
Place these tests into TestMyCut.t.sol

function test_TotalRewardsBreaksContract() public mintAndApproveTokens{
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 3);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
vm.startPrank(player2);
vm.expectRevert();
Pot(contest).claimCut();
vm.stopPrank();
}
​
function test_TotalRewardsBreaksContract_2() public mintAndApproveTokens{
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 2);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
vm.expectRevert();
Pot(contest).claimCut();
vm.stopPrank();
}
​
function test_TotalRewards() public mintAndApproveTokens{
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 3);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player2);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest); // doesnt break as works on ratio system , 10% of remaining balance to owner , rest to claimers.
vm.stopPrank();
}
​
function test_TotalRewards_2() public mintAndApproveTokens{
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 100);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
vm.startPrank(player2);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.warp(91 days);
​
vm.startPrank(user);
ContestManager(conMan).closeContest(contest); // doesnt break as works on ratio system , 10% of remaining balance to owner , rest to claimers.
vm.stopPrank();
}

Tools Used

Manual review , Foundry tests

Recommendations

Add a check to see if sum of values of rewards array equals totalRewards in ContestManager

+ error ContestManager__TotalRewardsIncorrect();
.
.
.
function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
Create a new Pot contract
​
+ uint256 sum = 0;
+ uint256 length = rewards.length;
+ for(uint i=0;i<length;i++){
+ sum+=rewards[i];
+ }
+ if(sum != totalRewards){
+ revert ContestManager__TotalRewardsIncorrect();
+ }
.
.
.
}
​
Updates

Lead Judging Commences

equious Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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