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

No validation that sum of rewards equals totalRewards causes Pot insolvency or locked funds

diegotrigueros

Root + Impact

Description

  • The Pot constructor accepts rewards[] and totalRewards as separate parameters but never validates that sum(rewards[i]) == totalRewards.

  • If sum(rewards) > totalRewards, the Pot is underfunded — later claimants' transfers will revert. If sum(rewards) < totalRewards, excess tokens are permanently locked with no way to recover them.

// 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;
​
for (uint256 i = 0; i < i_players.length; i++) {
@> playersToRewards[i_players[i]] = i_rewards[i]; // no sum validation
}
}

Risk

Likelihood:

  • The owner passes totalRewards manually through createContest(). A simple miscalculation or off-by-one error in the frontend or script causes a mismatch.

  • There is no on-chain check to prevent this, so every contest creation relies on off-chain correctness.

Impact:

  • If sum(rewards) > totalRewards: the Pot is underfunded. The remainingRewards tracker underflows or the actual token balance runs out. Last claimants cannot receive their rewards.

  • If sum(rewards) < totalRewards: remainingRewards never reaches 0 even after all claims. Excess tokens sit in the Pot forever with no recovery function.

  • The closePot function compounds this issue by distributing based on the inflated remainingRewards value.

Proof of Concept

The following test creates a Pot where the sum of individual rewards (120 ether) exceeds totalRewards (100 ether). The first two players drain the available balance, and the third player's claim reverts because there are not enough tokens remaining.

function testRewardsMismatch() public {
address[] memory players = new address[](3);
uint256[] memory rewards = new uint256[](3);
players[0] = address(0x1); rewards[0] = 40 ether;
players[1] = address(0x2); rewards[1] = 40 ether;
players[2] = address(0x3); rewards[2] = 40 ether;
// sum(rewards) = 120 ether, but totalRewards = 100 ether
​
Pot pot = new Pot(players, rewards, token, 100 ether);
token.transfer(address(pot), 100 ether);
​
// Player 1 claims 40, Player 2 claims 40 — 80 out, 20 remain
vm.prank(address(0x1)); pot.claimCut();
vm.prank(address(0x2)); pot.claimCut();
​
// Player 3 tries to claim 40 but only 20 tokens left
vm.prank(address(0x3));
vm.expectRevert(); // insufficient balance
pot.claimCut();
}

Recommended Mitigation

Add a validation loop in the Pot constructor that sums all individual rewards and requires the total to equal the totalRewards parameter. Also validate that players and rewards arrays have the same length to prevent out-of-bounds access or silently ignored entries.

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ require(players.length == rewards.length, "Length mismatch");
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;
i_deployedAt = block.timestamp;
​
+ uint256 rewardSum;
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
+ rewardSum += i_rewards[i];
}
+ require(rewardSum == totalRewards, "Rewards sum mismatch");
}
Updates

Lead Judging Commences

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