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

ContestManager::createContest uses unbounded arrays and omits totalRewards validation, enabling gas-limit DoS and underfunded pots where some players can never claim

virgilbb

Root + Impact

Description

  • ContestManager::createContest deploys a Pot contract that maps each player to their individual reward amount, so every player can later call claimCut() to receive their share of the deposited tokens.

  • Two independent defects make this logic inefficient and unsafe: (1) the Pot constructor iterates over all players in a loop to populate playersToRewards, so a large players[] array can exceed the block gas limit and make the pot undeployable; and (2) createContest never validates that the sum of rewards[] equals totalRewards, so a manager can deploy a pot that is funded with fewer tokens than the rewards map promises, leaving later claimants unable to collect.

// src/ContestManager.sol — createContest()
function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
// @> no check that sum(rewards) == totalRewards; pot can be underfunded
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}
// src/Pot.sol — 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;
​
// @> unbounded loop: large players[] array can hit block gas limit
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}

Risk

Likelihood:

  • Any contest with a sufficiently large player set will fail to deploy because the constructor loop runs on-chain during contract creation, consuming gas proportional to the number of players.

  • A manager entering totalRewards manually (e.g. a round number for UI display) is likely to supply a value that does not exactly match the computed sum of rewards[], and the protocol provides no on-chain guard to catch the discrepancy before the pot is funded.

Impact:

  • If sum(rewards) < totalRewards, the pot is underfunded: early claimants succeed while later claimants exhaust the balance and their claimCut() calls revert on the ERC-20 transfer, permanently losing their entitlement.

  • If the player array is large enough to exceed the block gas limit, the Pot constructor reverts and the contest cannot be created at all, denying the service entirely.

Proof of Concept

The test below shows the underfunded scenario directly. The manager passes totalRewards = 6 (the contest's intended reward constant) while the rewards[] array sums to more than what fundContest actually transfers. Player 1 claims successfully; Player 2 cannot because the pot is drained.

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 pot is underfunded
vm.expectRevert();
Pot(contest).claimCut();
vm.stopPrank();
}

player2's claimCut() reverts because the ERC-20 balance of the pot has already been exhausted by player1's claim, confirming that the missing totalRewards validation leaves late claimants with no recourse.

Recommended Mitigation

Add a sum-validation step in createContest before deploying the Pot, and document a player-count cap to guard against the gas-limit DoS in the constructor loop.

+ error ContestManager__RewardSumMismatch();
​
function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
+ uint256 rewardSum;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ rewardSum += rewards[i];
+ }
+ if (rewardSum != totalRewards) revert ContestManager__RewardSumMismatch();
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}

For the gas-DoS vector, consider replacing the constructor loop with a lazy-initialization pattern (e.g. mapping populated via a separate addPlayers batched call with a capped batch size) so that no single transaction must iterate the full player set.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 15 days 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!