MyCut

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

Unvalidated totalRewards allows underfunded claims and reverts

Root + Impact

Description

  • Normal behavior: `totalRewards` should match the sum of per-player rewards to ensure all claims succeed.

  • Issue: `totalRewards` is accepted without validation, causing underflows or leftover funds.

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;
...
@> for (uint256 i = 0; i < i_players.length; i++) {
@> playersToRewards[i_players[i]] = i_rewards[i];
}
}
function claimCut() public {
...
@> remainingRewards -= reward;
...
}

Risk

Likelihood:

  • Contest parameters are configured with a mismatch between totalRewards and sum of rewards.

  • Multiple claimants exercise their rewards.

Impact:

  • Later claims revert due to underflow.

  • Remaining funds become locked or distribution becomes inconsistent.

Proof of Concept

function testTotalRewardsMismatchRevertsOnLaterClaim() public {
address owner = makeAddr("owner");
address p1 = makeAddr("p1");
address p2 = makeAddr("p2");
ContestManager manager = _deployManager(owner);
ERC20Mock token = new ERC20Mock("T", "T", owner, 0);
token.mint(owner, 100);
vm.prank(owner);
token.approve(address(manager), 100);
address[] memory players = new address[](2);
players[0] = p1;
players[1] = p2;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 60;
rewards[1] = 60;
vm.startPrank(owner);
address potAddr = manager.createContest(players, rewards, IERC20(token), 100);
manager.fundContest(0);
vm.stopPrank();
vm.prank(p1);
Pot(potAddr).claimCut();
vm.prank(p2);
vm.expectRevert(stdError.arithmeticError);
Pot(potAddr).claimCut();
}

Recommended Mitigation

+ require(players.length == rewards.length, "length mismatch");
+ uint256 sum;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ sum += rewards[i];
+ }
+ require(sum == totalRewards, "total 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!