MyCut

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

totalRewards is caller-supplied and never validated against the sum of individual reward allocations

Root + Impact

Description

createContest() accepts a caller-supplied totalRewards independently of the rewards[] array. There is no on-chain check that totalRewards == sum(rewards). This creates two failure modes: if totalRewards < sum(rewards), the Pot runs out of tokens mid-distribution and reverts for later claimants with no recourse; if totalRewards > sum(rewards), the excess is funded but bounded by the constructor snapshot and is permanently unclaimable.

// ContestManager.sol :: createContest()
function createContest(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
// @> Caller-supplied, never cross-checked against sum(rewards)
uint256 totalRewards
) public onlyOwner returns (address) {
Pot pot = new Pot(players, rewards, token, totalRewards);
// ...
}
// Pot.sol :: constructor()
// @> totalRewards stored as-is, no sum validation performed
remainingRewards = totalRewards;

Risk

Likelihood:

  • Manual contest setup is error-prone — an operator who edits the rewards array after computing totalRewards passes a stale value with no on-chain safety net.

  • Off-chain tooling that computes totalRewards separately from the rewards array introduces a desync surface that will eventually produce a mismatch at scale.

Impact:

  • Under-funded case: players who claim later in the window find the Pot has insufficient balance — their claimCut() reverts and they permanently lose their allocation.

  • Over-funded case: tokens beyond the claimable total are permanently locked in the Pot with no recovery path.

Proof of Concept

// rewards = [500e18, 500e18] → sum = 1_000e18
// totalRewards = 800e18 (typo / off-chain bug)
// fundContest() transfers 800e18 into Pot
// Player 1 claims 500e18 → succeeds (800 - 500 = 300 remaining)
// Player 2 claims 500e18 → REVERTS (only 300e18 in contract)
// Player 2 permanently locked out of their allocation

Recommended Mitigation

// Pot.sol :: constructor() — derive totalRewards on-chain, remove the parameter
- constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ constructor(address[] memory players, uint256[] memory rewards, IERC20 token) {
+ require(players.length == rewards.length, "Length mismatch");
+ uint256 computed;
+ for (uint256 i = 0; i < rewards.length; i++) { computed += rewards[i]; }
+ i_totalRewards = computed;
+ remainingRewards = computed;
// ...
}
// ContestManager.sol :: createContest() — remove totalRewards param
- uint256 totalRewards
- Pot pot = new Pot(players, rewards, token, totalRewards);
- contestToTotalRewards[address(pot)] = totalRewards;
+ Pot pot = new Pot(players, rewards, token);
+ contestToTotalRewards[address(pot)] = pot.getTotalRewards();
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!