MyCut

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

`totalRewards` is not validated against the sum of individual rewards — later claimants can't claim if it's too low

Root + Impact

Description

Neither createContest nor the Pot constructor checks that totalRewards actually equals the sum of the rewards array. If the admin passes a totalRewards value lower than the sum, the pot gets funded with fewer tokens than players are owed. Early claimants get their full reward, but later ones hit a revert because the pot doesn't have enough tokens left.

// src/Pot.sol — constructor
i_totalRewards = totalRewards; // @> taken at face value
remainingRewards = totalRewards;
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i]; // @> individual rewards set independently
}
// @> no check that sum(i_rewards) == totalRewards

Risk

Likelihood: The admin is trusted, but mistakes happen. One wrong number in the totalRewards parameter and the pot silently becomes insolvent. There's no validation to catch it.

Impact: Players who claim later get a revert — their reward is mapped but the tokens aren't there. If totalRewards is higher than the sum, the excess tokens are stuck forever with no way to extract them.


Proof of Concept

2 players, 600 each (sum = 1200), but totalRewards set to 1000. Player1 claims fine, player2's claim reverts.

function testM01_TotalRewardsMismatch() public {
address[] memory players = new address[](2);
players[0] = player1; players[1] = player2;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 600e18; rewards[1] = 600e18;
vm.startPrank(owner);
address contest = conMan.createContest(players, rewards, IERC20(weth), 1000e18);
conMan.fundContest(0);
vm.stopPrank();
vm.prank(player1);
Pot(contest).claimCut(); // succeeds, gets 600
vm.prank(player2);
vm.expectRevert(); // pot only has 400, can't send 600
Pot(contest).claimCut();
}

Recommended Mitigation

The constructor should compute the actual sum of individual rewards and verify it matches totalRewards before storing anything. This catches mismatches at deployment time — before the pot is funded — so no tokens can be sent to an insolvent or over-funded pot. The array length check (see L-01) should also be included here to catch related input errors in the same place.

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ require(players.length == rewards.length, "Array length mismatch");
+ uint256 rewardsSum;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ rewardsSum += rewards[i];
+ }
+ require(rewardsSum == totalRewards, "Rewards don't match totalRewards");
+
i_players = players;
Updates

Lead Judging Commences

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