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

Missing Total Rewards Validation

r7al38

Root + Impact

Description

  • Normally, when creating a contest, the owner provides an array of rewards for each player and a totalRewards value that should represent the sum of those individual rewards. The contract should ensure that the total amount funded matches the cumulative prizes to guarantee that every player can claim their reward.


  • The issue is that ContestManager.createContest() accepts a totalRewards parameter without validating that it equals the sum of the rewards array. This allows the owner to accidentally (or maliciously) input a value that differs from the actual total required, leading to underfunding of the contest.

function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
// Create a new Pot contract
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards; // @> no check that totalRewards == sum(rewards)
return address(pot);
}

Risk

Likelihood:Medium

  • Owner error – a simple typo when entering the total can easily happen.

  • Malicious owner – an untrusted owner could intentionally set a lower total to steal funds or disrupt the contest.

Impact:High

  • Players cannot claim their full rewards, leading to a broken user experience and loss of funds.

  • The contest becomes unfair and damages the protocol's reputation; funds may remain stuck in the contract.

Proof of Concept

The following Foundry test demonstrates the issue:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import "forge-std/Test.sol";
import "../src/ContestManager.sol";
import "../src/Pot.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
// ERC20
contract MockERC20 is ERC20 {
constructor() ERC20("Mock", "MCK") {
_mint(msg.sender, 1000 * 10 ** 18);
}
}
​
contract ContestManagerTest is Test {
ContestManager contestManager;
MockERC20 token;
address owner = address(0x123);
address player1 = address(0x1);
address player2 = address(0x2);
address player3 = address(0x3);
​
function setUp() public {
vm.startPrank(owner);
contestManager = new ContestManager();
token = new MockERC20();
vm.stopPrank();
}
​
function testTotalRewardsMismatch() public {
address[] memory players = new address[](3);
players[0] = player1;
players[1] = player2;
players[2] = player3;
​
uint256[] memory rewards = new uint256[](3);
rewards[0] = 100;
rewards[1] = 100;
rewards[2] = 100;
​
uint256 totalRewards = 200; // Value that does not match the true total
​
// 1. Creating the competition – must pass without revert (no verification)
vm.prank(owner);
address potAddress = contestManager.createContest(players, rewards, IERC20(address(token)), totalRewards);
​
// 2.Verify that the totalRewards stored in ContestManager is 200 and not 300
uint256 storedTotal = contestManager.contestToTotalRewards(potAddress);
assertEq(storedTotal, 200, "contestToTotalRewards should store the input value 200");
​
// 3. The competition will be funded with the entered amount (200)
vm.prank(owner);
token.approve(address(contestManager), totalRewards);
​
vm.prank(owner);
contestManager.fundContest(0); // index 0
​
// 4. Checking the Pot contract balance after funding
uint256 potBalance = token.balanceOf(potAddress);
assertEq(potBalance, 200, "Pot should have 200 tokens after funding");
​
// 5. Calculate the total actual prizes (must be 300)
uint256 actualTotal = 0;
for (uint i = 0; i < rewards.length; i++) {
actualTotal += rewards[i];
}
assertEq(actualTotal, 300, "Actual total rewards should be 300");
​
// 6. Prove that the balance in Pot is less than required (200 < 300)
assertLt(potBalance, actualTotal, "Pot balance is less than required total rewards");
​
//This proves that the contract will not be able to fulfill all withdrawal requests, resulting in one player being unable to claim their prize
}
}
​

The test passes, proving that the third player cannot claim his reward due to the mismatch.

Recommended Mitigation

Add a validation step that computes the sum of rewards and reverts if it does not equal totalRewards.

function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)
public
onlyOwner
returns (address)
{
+ uint256 sum = 0;
+ for (uint i = 0; i < rewards.length; i++) {
+ sum += rewards[i];
+ }
+ require(totalRewards == sum, "totalRewards must equal sum of rewards");
​
Pot pot = new Pot(players, rewards, token, totalRewards);
contests.push(address(pot));
contestToTotalRewards[address(pot)] = totalRewards;
return address(pot);
}
Updates

Lead Judging Commences

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