MyCut

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

Missing Validation of Reward Allocations Can Cause Underfunded Contest Pots

Root + Impact

The Pot constructor accepts players, rewards, and totalRewards without verifying that the individual reward allocations match the total funded amount. If the sum of the promised rewards exceeds totalRewards, eligible players may be unable to withdraw their full allocations because the pot can run out of tokens. This can leave some participants unpaid. The issue occurs when the contest owner creates a pot with inconsistent reward allocations. It is conditional on the supplied values and the order in which participants claim their rewards.

Description

The constructor stores the supplied reward amounts without validating their sum against totalRewards

The ContestManager subsequently funds the pot with totalRewards, but neither contract verifies that the total reward allocations are covered by that amount.

For example, a pot may be configured with two players who are each promised 100 tokens while totalRewards is only 100 tokens. The first successful claim transfers 100 tokens, leaving the second player's claim unfunded.

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;
​
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i];
}
}

Risk

Likelihood: The problem depends on incorrect or inconsistent reward values supplied during contest creation.

Impact: Some players may be unable to receive their promised rewards because the pot lacks sufficient tokens.

Proof of Concept

Note: This test assumes the existing test environment provides vm, a compatible ERC20 mock named token, and an ERC20 implementation that reverts on transfers exceeding the available balance. Adapt the setup to the project's actual test fixtures.

function testUnderfundedRewardAllocations() public {
address[] memory players = new address[](2);
players[0] = address(0xA1);
players[1] = address(0xB2);
​
uint256[] memory rewards = new uint256[](2);
rewards[0] = 100;
rewards[1] = 100;
​
uint256 totalRewards = 100;
​
Pot pot = new Pot(
players,
rewards,
IERC20(address(token)),
totalRewards
);
​
// Fund the pot with only 100 tokens.
token.mint(address(this), 100);
token.approve(address(pot), 100);
token.transfer(address(pot), 100);
​
// First player claims the entire pot balance.
vm.prank(players[0]);
pot.claimCut();
​
// The second player's promised reward is still 100,
// but the pot has no tokens remaining.
assertEq(pot.checkCut(players[1]), 100);
assertEq(token.balanceOf(address(pot)), 0);
​
// The second claim cannot be paid by a standard ERC20
// that reverts when the sender has insufficient balance.
vm.expectRevert();
vm.prank(players[1]);
pot.claimCut();
}

Recommended Mitigation

Validate the inputs before storing reward allocations. Require the player and reward arrays to have equal lengths, reject duplicate player addresses if each player should have only one allocation, and ensure the total allocated rewards do not exceed the amount intended to be funded.

If the protocol intends to allocate the entire pot to players, require allocatedRewards == totalRewards instead. Ensure the contest is funded before allowing claims, and test that all configured allocations can be paid successfully.

constructor(
address[] memory players,
uint256[] memory rewards,
IERC20 token,
uint256 totalRewards
) {
+ require(players.length == rewards.length, "Length mismatch");
+
+ uint256 allocatedRewards;
+ for (uint256 i = 0; i < rewards.length; i++) {
+ allocatedRewards += rewards[i];
+ }
+ require(
+ allocatedRewards <= totalRewards,
+ "Insufficient total rewards"
+ );
+
i_players = players;
i_rewards = rewards;
i_token = token;
i_totalRewards = totalRewards;
remainingRewards = 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!