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

No implementation to ensure that `totalRewards` is at least the sum of `rewards` array items in the `Pot` contract constructor

soupy

Summary

No amount validity check in the Pot contract constructor to ensure the totalRewards is at least the sum of rewards array items. The lack of this amount check will cause the protocol to fail with arithmetic underflow or overflow error.

https://github.com/Cyfrin/2024-08-MyCut/blob/946231db0fe717039429a11706717be568d03b54/src/Pot.sol#L22-L35

Vulnerability Details

The protocol is a reward distribution contract with players array, rewards array and totalRewards being defined as contract constructor arguments. For the reward system to be valid, the totalRewards value has to be at least the sum of rewards array items so every player who later on makes the claim will receive their rewards. However, this check is currently not implemented in the Pot constructor, causing the protocol to face the risk of arithmetic underflow or overflow failure.

Proof of Concept:

In test/TestMyCut.t.sol, add the following test case:

function test_audit_totalRewardsLessThanSumOfRewardsArrayItems() public mintAndApproveTokens {
// the following are already initialized and tapout in the main Test setup
// address[] players = [player1, player2];
// uint256[] rewards = [3, 1];
​
// change the initial totalRewards from 4 down to 3
uint256 totalRewards = 3;
​
vm.startPrank(user);
contest = ContestManager(conMan).createContest(players, rewards, IERC20(weth), totalRewards);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.prank(player1);
Pot(contest).claimCut();
​
vm.prank(player2);
Pot(contest).claimCut();
}

The test run will fail and revert with arithmetic underflow or overflow error when the totalRewards is less than the sum of rewards array items.

Impact

The protocol will fail to execute the reward distribution properly for all its eligible players and affects their reputation and credibility.

Tools Used

Manual review

Recommendations

To implement amount check in the Pot constructor as demonstrated below:

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
​
+ uint256 rewardsSum = 0;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ rewardsSum += rewards[i];
+ }
+
+ if (totalRewards < rewardsSum) {
+ revert Pot__totalRewardsLessThanRewardsSum();
+ }
​
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
//@audit-l: should avoid using block.timestamp as miner may manipulate the time to delay owner to closePot to lengthen the period before owner close the pot
i_deployedAt = block.timestamp;
​
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}
Updates

Lead Judging Commences

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

Support

FAQs

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