MyCut

AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Submission Details
Severity: low
Valid

createContest does not validate that rewards array sum equals totalRewards, allowing underfunded or overfunded pots

Root + Impact

Description

  • createContest accepts both a rewards array and a totalRewards value but never validates that they match. The rewards array is used to set individual player allocations in playersToRewards while totalRewards is used to fund the pot. If the sum of rewards exceeds totalRewards, players who claim later will cause the pot to revert due to insufficient balance. If the sum is less than totalRewards, excess funds are permanently locked.

function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public onlyOwner returns (address)
{
Pot pot = new Pot(players, rewards, token, totalRewards);
// No validation: sum(rewards) == totalRewards
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}

Risk

Likelihood:

  • Human error in specifying rewards vs totalRewards is common

  • No on-chain enforcement means this can be deployed in broken state

Impact:

  • If sum(rewards) > totalRewards: later claimants receive nothing, transactions revert

  • If sum(rewards) < totalRewards: excess funds permanently locked with no recovery

Proof of Concept

The following demonstrates a pot where individual rewards sum to more than totalRewards, causing the last
claimant to always fail:
function testRewardsMismatchCausesRevert() public {
address[] memory players = new address[](2);
players[0] = player1;
players[1] = player2;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 600e18; // sum = 1100e18
rewards[1] = 500e18;
// totalRewards only 1000e18 - underfunded vs rewards array
address potAddr = contestManager.createContest(players, rewards, token, 1000e18);
contestManager.fundContest(0);
vm.prank(player1);
Pot(potAddr).claimCut(); // succeeds
vm.prank(player2);
vm.expectRevert(); // fails - insufficient balance
Pot(potAddr).claimCut();
}

Recommended Mitigation

function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public onlyOwner returns (address)
{
+ uint256 rewardsSum = 0;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ rewardsSum += rewards[i];
+ }
+ require(rewardsSum == totalRewards, "Rewards sum must equal totalRewards");
Pot pot = new Pot(players, rewards, token, totalRewards);
...
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[L-01] The logic for ContestManager::createContest is NOT efficient

## Description 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. ```Solidity 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` ```solidity 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 ## 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.

Support

FAQs

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

Give us feedback!