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

No validation that sum(rewards) equals totalRewards — pot can be under-funded or over-funded versus player allocations

virgilbb

Root + Impact

Description

  • createContest() accepts both a rewards array (individual player allocations summing to the full prize pool) and a totalRewards scalar (the amount actually transferred into the pot by fundContest()). The two values are intended to be equal.

  • No check enforces sum(rewards) == totalRewards. In the underfunded case (sum(rewards) > totalRewards) the first claimants exhaust the balance and later claimants cannot claim, permanently losing their allocation. In the overfunded case (sum(rewards) < totalRewards) the surplus inflates remainingRewards, causing the manager cut and claimant bonus to be calculated on tokens that should never have been part of those distributions.

function createContest(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) public onlyOwner returns (address) {
// no validation: sum(rewards) == totalRewards
Pot pot = new Pot(players, rewards, token, totalRewards);

Risk

Likelihood:

  • Any manual configuration error where the rewards array and totalRewards are set independently (different spreadsheet cells, off-by-one in a script, decimal precision mismatch) produces this inconsistency silently at deployment.

  • The lack of any revert means misconfiguration goes undetected until players start claiming and either fail or receive unexpected amounts.

Impact:

  • Underfunded scenario: the last claimants in call order find the pot balance exhausted and cannot claim. Their allocation is not refunded — it flows into the close distribution as a fractional "remaining rewards" share rather than the full entitled amount, causing direct, irreversible token loss.

  • Overfunded scenario: the manager receives a cut on surplus tokens that were never part of the intended prize pool, and claimants receive a bonus inflated by the overcollection.

Proof of Concept

Place this test in test/ and run forge test --match-test testUnderfundedPotCausesClaimFailure. The test demonstrates that a Pot deployed with a totalRewards argument that doesn't match the sum of the rewards array causes claimCut() to revert for later claimants once the contract balance is exhausted. The ordering is enforced: player2 (1-token allocation) claims first and succeeds; player1 (5-token allocation) claims second and reverts because only 3 tokens remain in the pot.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test} from "forge-std/Test.sol";
import {ContestManager} from "src/ContestManager.sol";
import {Pot} from "src/Pot.sol";
import {MockERC20} from "test/mocks/MockERC20.sol";
​
contract H4Test is Test {
MockERC20 token;
ContestManager cm;
​
address owner = address(0x1);
address player1 = address(0x2);
address player2 = address(0x3);
​
function setUp() public {
token = new MockERC20();
vm.prank(owner);
cm = new ContestManager();
}
​
function testUnderfundedPotCausesClaimFailure() public {
address[] memory players = new address[](2);
players[0] = player1;
players[1] = player2;
​
// Rewards sum to 6 tokens; totalRewards is only 4 tokens (underfunded by 2)
uint256[] memory rewards = new uint256[](2);
rewards[0] = 5e18; // player1 allocated 5 tokens (cannot be satisfied)
rewards[1] = 1e18; // player2 allocated 1 token (fits within pot balance)
​
token.mint(owner, 4e18); // only enough for totalRewards = 4
​
vm.startPrank(owner);
cm.createContest(players, rewards, token, 4e18); // sum(rewards)=6 != totalRewards=4, no revert
token.approve(address(cm), 4e18);
cm.fundContest(0);
vm.stopPrank();
​
address pot = cm.contests(0);
​
// player2 claims first (1 token) — succeeds because pot still holds 4 tokens
vm.prank(player2);
Pot(pot).claimCut();
​
// assert player2 received their full allocation
assertEq(token.balanceOf(player2), 1e18, "player2 should have received 1 token");
​
// player1 tries to claim 5 tokens but pot now holds only 3 — transfer reverts
vm.prank(player1);
vm.expectRevert();
Pot(pot).claimCut();
​
// assert player1 received nothing — allocation permanently unclaimable
assertEq(token.balanceOf(player1), 0, "player1 should have received nothing");
}
}

Recommended Mitigation

Add a constructor-time validation that sums the rewards array and asserts it equals totalRewards, reverting deployment if the values are inconsistent.

+ uint256 rewardSum;
+ for (uint256 i = 0; i < players.length; i++) {
+ rewardSum += rewards[i];
+ }
+ require(rewardSum == totalRewards, "rewards sum mismatch");
+ require(players.length == rewards.length, "array length mismatch");
Pot pot = new Pot(players, rewards, token, totalRewards);
Updates

Lead Judging Commences

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