MyCut

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

## M-2 — `Σ rewards != totalRewards` is never validated

Root + Impact

remainingRewards is initialized to totalRewards and decremented by each player's configured reward. Nothing enforces sum(rewards) == totalRewards. If the sum exceeds totalRewards, earlier claimants drain the pot and the final claim underflows; if it is less, the surplus becomes unclaimable residual.

Description

// src/Pot.sol
remainingRewards = totalRewards; // @> seeded, unvalidated against sum(rewards)
...
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
...
function claimCut() public {
...
remainingRewards -= reward; // @> underflows when sum(rewards) > totalRewards
...
}
// src/ContestManager.sol
function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public onlyOwner returns (address)
{
// @> no check: sum(rewards) == totalRewards
}

Risk

Likelihood: Any misconfigured contest (rewards not summing to totalRewards).

Impact: Last claimants cannot withdraw (checked underflow / balance shortfall); surplus is stranded.

Proof of Concept

// test/Exploits.t.sol
function testRewardsExceedTotalRewards() public {
// rewards sum = 200 but only 150 funded
Pot pot = _createFundClose(100e18, 100e18, 150e18);
​
vm.prank(alice);
pot.claimCut(); // remaining 150 -> 50
​
vm.prank(bob);
vm.expectRevert(); // 50 - 100 underflows
pot.claimCut();
}

Recommended Mitigation

+ uint256 sum;
+ for (uint256 i = 0; i < rewards.length; i++) sum += rewards[i];
+ require(sum == totalRewards, "rewards must sum to totalRewards");
Pot pot = new Pot(players, rewards, token, totalRewards);

Updates

Lead Judging Commences

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