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

No checks on whether or not totalRewards is actually the sum of all rewards

ollam

Summary

In ContestManager::createContest(), rewards[] and totalRewards are both provided by the protocol manager. However, there are no checks to ensure that these values reflect each other, which can lead to numerous issues with the actual contest.

Vulnerability Details

Please view the following two functions:

// ContestManager.sol
function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}
// Pot.sol
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
​
// i_token.transfer(address(this), i_totalRewards);
​
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}

There are no checks to ensure that totalRewards is actually the sum of all rewards.

Impact

This would be a fairly simple mistake to make when creating a new Pot, especially if the length of the players and rewards arrays are very long. If totalRewards is less than the sum of rewards[], then it would be a race to make claims because later claimants would run the risk of the Pot running out of funds. Additionally, if totalRewards is too small for any claimant to claim any rewards, 90% of funds will be stuck at the end of the contest.

PoC - paste this into TestMyCut.t.sol

This creates a contest in which totalSupply is 1, while player1 is due 3.

function testNotEnoughTotalRewards() public mintAndApproveTokens {
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 1);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
uint256 balanceBefore = ERC20Mock(weth).balanceOf(player1);
vm.startPrank(player1);
vm.expectRevert();
Pot(contest).claimCut();
vm.stopPrank();
}

Tools Used

Manual review

Recommendations

Add a check to make sure that totalRewards is at least equal to the sum of rewards[] to avoid breaking the Pot's functionallity.

Alternatively, if there is no desired instance in which totalRewards is greater than the sum of rewards[], remove it as an argument and calculate it within the contract itself from rewards[].

Updates

Lead Judging Commences

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

Support

FAQs

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