MyCut

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

`ContestManager::createContest` trusts an unvalidated `totalRewards` and uses unbounded arrays

Root + Impact

Description

  • `createContest` accepts `players`, `rewards`, and a separate `totalRewards`, and forwards them straight into the `Pot` constructor without validating that `players.length == rewards.length` or that `Σ rewards == totalRewards`.

  • If `totalRewards` is set lower than the sum of `rewards`, the `Pot` is funded for less than the total owed, so later claimants revert once the balance is exhausted (some players can never claim). Additionally, `players`/`rewards` are unbounded dynamic arrays copied in a constructor loop, so an oversized list can make deployment prohibitively expensive.

// src/ContestManager.sol
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 length / sum validation
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}
// src/Pot.sol :: constructor
@> for (uint256 i = 0; i < i_players.length; i++) { // unbounded copy loop
playersToRewards[i_players[i]] = i_rewards[i]; // reverts if rewards.length < players.length
}

Risk

Likelihood:

  • Requires an admin input mistake (wrong `totalRewards`, mismatched array lengths, or an oversized array). Trusted-role dependent, hence Low likelihood.


Impact:

  • Under-set `totalRewards` → some players can never claim (funds owed but not funded).

  • Mismatched lengths → constructor reverts (griefed deployment) or misaligned rewards.

  • Oversized arrays → deployment may be too gas-heavy to execute.

Proof of Concept


N/A

Recommended Mitigation


Validate inputs in the constructor (and derive `totalRewards` instead of trusting it):

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ require(players.length == rewards.length, "length mismatch");
+ uint256 sum;
+ for (uint256 i = 0; i < rewards.length; i++) { sum += rewards[i]; }
+ require(sum == totalRewards, "totalRewards mismatch");
i_players = players;
i_rewards = rewards;
...
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 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!