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

Missing check totalRewards and total array of rewards in `Pot.sol` constructor

hrmneffdii

Summary

In Pot.sol, there is no checking for totalRewards and array of rewards. If the totalRewards more higher than total array of rewards, it will cause the protocol has a rest of rewards. also if the totalRewards more lower than total array of rewards, it will cause the last player can not take the reward because rewardsRemaining unsufficient.

Vulnerability Details

Let's se the following codebase below :

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 isn't any checking for totalRewards and total array of rewards. There is not guarantee if totalRewards and total of array rewards is exactly same. There is PoC below this :

function testForTotalRewardsLessThanArrayOfRewardMakeOverOrUnderFlow() public {
address[] memory playerss = new address[](2);
playerss[0] = makeAddr("p0");
playerss[1] = makeAddr("p1");
​
uint256[] memory rewardss = new uint256[](2);
rewardss[0] = 10;
rewardss[1] = 20;
​
uint256 totalRewardss = 20;
vm.startPrank(user);
contest = ContestManager(conMan).createContest(playerss, rewardss, IERC20(ERC20Mock(weth)), totalRewardss);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
// playerss[0] will claim his rewards
// totalRewards - rewards of playerss[0]
// 20 - 10 = 10 remaining token
vm.prank(playerss[0]);
Pot(contest).claimCut();
​
// playerss[1] will claim his rewards
// totalRewards - rewards of playerss[0]
// 10 - 20 = underflow
vm.prank(playerss[1]);
vm.expectRevert();
Pot(contest).claimCut();
}

Impact

The impact that willl occur either the protocol still have a rest of rewards whereas all of players claimed their rewards or the player who take claim in last will not occurs because rewards remaining unsufficient to charge the player.

Tools Used

Manual review ~ Foundry

Recommendations

Add checking for Pot.sol constructor following this :

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);
+ uint256 totalRewardsManual;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ totalRewardsManual += rewards[i];
+ }
+ require(totalRewardsManual == totalRewards);
​
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: Other

Support

FAQs

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