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

DOS while creating Contest for Large NUmber of Players

daspra0tik

Root + Impact

Description

  • The function createContest is made to create a contest for any number of player by the owner as no max player is set.

  • The issue arrises when a huge number of Players participated in the contest which arises DOS vulnerability.

  • No MAXimum number of player is set.

function createContest(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) public onlyOwner returns (address) {
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}

Risk

Say the owner creates a contest and sets totalRewards to 1e15.

  • The owner calls createContest() — since the token transfer line is commented out, no tokens ever leave the owner's wallet, and no tokens ever reach the Pot contract.

  • The result: Pot's storage records totalRewards = 1e15 and remainingRewards = 1e15, but Pot's actual ERC20 token balance is 0. The contract's internal accounting is now completely disconnected from reality — it believes it holds funds it never received.

Impact:

Since createContest() never actually transfers totalRewards tokens into Pot, the contract becomes insolvent from the moment it's deployed, despite its storage claiming otherwise.

  • Any call to claimCut() will revert when _transferReward attempts to move tokens the contract doesn't have.

  • closePot() will similarly revert (or silently fail to pay out) for the same reason.

  • Players are misled into believing rewards are available (since checkCut() and remainingRewards report nonzero values), when in fact nothing can ever be claimed unless the Pot is separately funded after deployment — which the current contracts provide no clean mechanism for.

Proof of Concept

function test_RevertIfLargeNumberofPlayerarePlaying() public {
// Arrange
uint256 totalPlayers = 20000;
address[] memory players = new address[](totalPlayers);
uint256[] memory rewards = new uint256[](totalPlayers);
​
// Tot al rewards needed for 20 players getting 1e18 each
uint256 totalRewards = totalPlayers * 1e18;
​
// Start acting as the contest creator
vm.startPrank(default_Foundry);
​
// 1. Give the creator the total funds and approve the manager
merc20.mint(default_Foundry, totalRewards);
merc20.approve(address(cmanager), totalRewards);
​
// Act
// 2. Loop starts at 0 to fill every slot in the array
for (uint256 i = 0; i < totalPlayers; i++) {
// i + 1 avoids generating address(0)
address a = address(uint160(i + 1));
​
players[i] = a;
rewards[i] = 1e18;
players[19999] = spider;
rewards[19999] = 1e18;
// Assign 1e18 reward to this specific player
}
​
ERC20Mock token = merc20;
​
// 3. Create the contest
vm.expectRevert();
cmanager.createContest(players, rewards, token, totalRewards);
vm.stopPrank();
}

Recommended Mitigation

  • Set a maximum length or limit of player to avoid DOS.

function createContest(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) public onlyOwner returns (address) {
// Create a new Pot contract
require(players.length < 10, "ToManyPlayersArePlaying");
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}
Updates

Lead Judging Commences

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