MyCut

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

Unvalidated totalRewards Parameter


Description

  • The Pot contract constructor accepts both a totalRewards parameter and an array of individual rewards that should sum to this total.

  • If the totalRewards parameter does not match the sum of the individual rewards array, it creates a mismatch that can lead to incorrect fund allocation or funds becoming stuck in the contract. Without validation, the contract could be deployed with inconsistent state where the declared total rewards does not equal the actual sum of what players should receive.

constructor(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) {
// @> Missing validation that totalRewards equals sum of rewards array
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards; // @> Could be mismatched with sum of rewards
remainingRewards = totalRewards;

Risk

Likelihood:

  • Constructor is called once during deployment, making it a one-time opportunity for the owner to provide incorrect parameters.

  • Owner error or incorrect off-chain calculations when preparing reward distribution could result in mismatched values being passed.

Impact:

  • If totalRewards > sum(rewards), excess funds remain locked in the contract after all valid claims are processed, potentially lost permanently.

  • If totalRewards < sum(rewards), some players cannot claim their full entitled rewards, causing fund insufficiency errors during legitimate withdrawal attempts.

Proof of Concept

// Owner deploys with mismatched parameters
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 100e18;
rewards[1] = 200e18;
// Sum of rewards = 300e18, but totalRewards is set incorrectly
uint256 totalRewards = 250e18; // Mismatch!
Pot pot = new Pot(players, rewards, token, totalRewards);
// bob cannot claim full reward, transaction reverts with insufficient funds

Recommended Mitigation

constructor(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) {
require(
players.length == rewards.length,
Pot__PlayersLengthDoesntMatchRewardsLength()
);
+ uint256 sumOfRewards = 0;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ sumOfRewards += rewards[i];
+ }
+
+ require(
+ sumOfRewards == totalRewards,
+ Pot__TotalRewardsDontMatchSumOfRewards()
+ );
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = totalRewards;- remove this code
+ add this code
Updates

Lead Judging Commences

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