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

Lack of prerequisite check to ensure contract `Pot` constructor arguments `players` and/or `rewards` arrays are not empty

soupy

Summary

No proper array validity check for contract Pot constructor arguments players and rewards array could cause the Pot:playersToRewards fails to map out properly which any player who calls the Pot:claimCut will be reverted with Pot__RewardNotFound() error, making it useless as reward distribution protocol.

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

Vulnerability Details

Contract Pot maps the players to rewards in its constructor. For this mapping, it depends on the constructor arguments array players and rewards. However, there is no array condition check currently to ensure these arrays are not zero length and their length size matches.

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;
​
for (uint256 i = 0; i < i_players.length; i++) {
<@@> playersToRewards[i_players[i]] = i_rewards[i];
}
}

Proof of Concept:

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

function test_audit_emptyPrayersRewardsConstructorArguments() public mintAndApproveTokens {
address[] memory emptyPlayers;
uint256[] memory emptyRewards;
vm.startPrank(user);
contest = ContestManager(conMan).createContest(emptyPlayers, emptyRewards, IERC20(weth), 4);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
​
vm.prank(player1);
vm.expectRevert(Pot.Pot__RewardNotFound.selector);
Pot(contest).claimCut();
}

Test run will pass indicating that empty array in the constructor arguments array Players and Rewards will eventually cause the protocol to revert with Pot__RewardNotFound when player calls the Pot:claimCut function.

Impact

Contract Pot fails to map out players to rewards and cause the protocol is not able to function as intended reward distribution protocol

Tools Used

Manual review

Recommendations

To include prerequisite check for the constructor argument players and rewards array in the Pot contract as demonstrated below :

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
​
+ if (players.length == 0) {
+ revert Pot__EmptyPlayers();
+ }
+
+ if (players.length != rewards.length) {
+ revert Pot__PlayersAndRewardsLengthMismatch();
+ }
​
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
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: Other

Support

FAQs

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