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

The logic for ContestManager::createContest is NOT efficient

cryptedoji

Summary

there are two major problems that comes with the way contests are created using the ContestManager::createContest.

  • using dynamic arrays for players and rewards leads to potential DoS for the Pot::constructor, this is possible if the arrays are too large therefore requiring too much gas

  • it is not safe to trust that totalRewards value supplied by the manager is accurate and that could lead to some players not being able to claimCut

Vulnerability Details

  • If the array of players is very large, the Pot::constructor will revert because of too much gas required to run the for loop in the constructor.

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];
@> }
}

  • Another issue is that, if a Pot is created with a wrong totalRewards that for instance is less than the sum of the reward in the rewards array, then some players may never get to claim their rewards because the Pot will be underfunded by the ContestManager::fundContest function.

PoC

Here is a test for wrong totalRewards

function testSomePlayersCannotClaimCut() public mintAndApproveTokens {
vm.startPrank(user);
// manager creates pot with a wrong(smaller) totalRewards value-
contest = ContestManager(conMan).createContest(players, rewards, IERC20(ERC20Mock(weth)), 6);
ContestManager(conMan).fundContest(0);
vm.stopPrank();
vm.startPrank(player1);
Pot(contest).claimCut();
vm.stopPrank();
​
vm.startPrank(player2);
// player 2 cannot claim cut because the pot is underfunded due to the wrong totalScore
vm.expectRevert();
Pot(contest).claimCut();
vm.stopPrank();
}

Impact

  • Pot not created if large dynamic array of players and rewards is used

  • wrong totlRewards value leads to players inability to claim their cut

Tools Used

  • manual review

  • foundry test

Recommendations

review the pot-creation design by, either using merkle tree to store the players and their rewards OR another solution is to use mapping to clearly map players to their reward and a special function to calculate the totalRewards each time a player is mapped to her reward. this totalRewards will be used later when claiming of rewards starts.

Updates

Lead Judging Commences

equious Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Unbound for loop in Contest Creation

Support

FAQs

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