MyCut

AI First Flight #8

Beginner FriendlyFoundry

EXP

View results

Previous Next

Submission Details

Impact: medium

Likelihood: low

Invalid

createContest and the Pot constructor never validate players, rewards and totalRewards (sum mismatch, duplicates, zero entries, empty list), so promised rewards become unclaimable, phantom remainders are paid out or the pot cannot close

friedsocks233

Missing input validation in `createContest` / `Pot` constructor + denied claims, misallocated bonus or a pot that can never close

Description

A contest pays rewards[i] to players[i], and the owner funds totalRewards. For the accounting to hold, five conditions must be met:
- the two arrays have equal length;
- players are unique and non-zero;
- rewards are non-zero;
- sum(rewards) == totalRewards;
- the player list is not empty.
The repo's own tests use consistent inputs, such as [3,1] for 4 and [500,500] for 1000.
Neither createContest nor the Pot constructor checks any of these conditions. As a result:
- remainingRewards is seeded from totalRewards independently of the rewards array.
- Duplicate players silently overwrite each other in playersToRewards.
- Every array entry, including zero-reward, zero-address and duplicate entries, counts in the i_players.length divisor that closePot uses.
- An empty players array makes that divisor zero.

// src/ContestManager.sol

16: function createContest(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards)

17: public

18: onlyOwner

19: returns (address)

20: {

21: // Create a new Pot contract

@> 22: Pot pot = new Pot(players, rewards, token, totalRewards);

23: contests.push(address(pot));

24: contestToTotalRewards[address(pot)] = totalRewards;

25: return address(pot);

26: }

// src/Pot.sol

22: constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {

23: i_players = players;

24: i_rewards = rewards;

25: i_token = token;

26: i_totalRewards = totalRewards;

@> 27: remainingRewards = totalRewards;

28: i_deployedAt = block.timestamp;

29:

30: // i_token.transfer(address(this), i_totalRewards);

31:

@> 32: for (uint256 i = 0; i < i_players.length; i++) {

@> 33: playersToRewards[i_players[i]] = i_rewards[i];

34: }

35: }

Risk

Likelihood:

Every limb happens when the trusted owner passes inconsistent parameters to createContest. Examples are a typo in the rewards array, a duplicated address in a long player list, or a totalRewards that differs from the sum of rewards. No unprivileged actor can trigger it.
Neither contract catches the slip, and the Pot has no setter, so the bad parameters stay for the life of the pot.

Impact:

Sum mismatch:
- With [600,600] against 1000, the later claimer's remainingRewards -= reward underflows (Panic 0x11). p2 receives 0 of the 600 promised, even after a second funding.
- With [300,300] against 1000, both players claim in full, yet 400 is treated as forfeited. 40 goes to the manager and each player gets a 180 bonus that nobody forfeited.
- A rewards array longer than players is silently ignored, and the extra amount becomes the same kind of phantom remainder.
Bad player entries:
- With [p1,p1,p2] and [500,200,300], p1's 500 is overwritten by 200 and can never be claimed. At close the divisor is 3 instead of 2, and 150 stays stuck.
- One zero-reward entry plus one zero-address entry halve a claimant's bonus (180 becomes 90).
- An empty players array on a funded pot makes every closeContest revert with division by zero (Panic 0x12), which locks all 1000 tokens.
- The divisor effects overlap with the separate i_players.length divisor issue. This report is about the missing validation.

Proof of Concept

The test deploys ContestManager and the repo's test/ERC20Mock.sol and goes through createContest, fundContest and closeContest. All eight tests pass:

test_baseline_consistentInputWorks (control): consistent input pays everyone and leaves the Pot empty.
test_limb1_sumAboveTotal_laterClaimerUnderflows: p2's claim reverts with stdError.arithmeticError, before and after a second funding. p2 ends with 0 while checkCut(p2) is 600.
test_limb2_sumBelowTotal_phantomRemainder: 400 is reported unclaimed although both players claimed. The manager takes 40 and each player gets 180 extra.
test_limb3_duplicatePlayers_overwriteAndDivisorInflation: checkCut(p1) is 200 and p1's second claim reverts with Pot__RewardNotFound. After close p1 has 350, p2 has 450, and 150 is stuck.
test_limb4_zeroEntriesShrinkBonus: the bonus is 180 with a clean list and 90 with the dead entries.
test_limb5_emptyPlayers_closeDivByZero_fundsLocked: closeContest reverts with stdError.divisionError at day 90 and again ten years later, and 1000 stays locked.
test_limb6a_shorterRewardsRevertsAtDeploy: deployment reverts with stdError.indexOOBError, which is the benign case.
test_limb6b_longerRewardsSilentlyIgnored: the extra 500 entry is ignored and paid out as a phantom remainder (manager 50, p1 950).

Save the file as test/PoC_MC-06.t.sol and run from the repository root:

FOUNDRY_FFI=false forge test --match-path test/PoC_MC-06.t.sol -vv

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

import {Test, console, stdError} from "lib/forge-std/src/Test.sol";

import {ContestManager} from "../src/ContestManager.sol";

import {Pot} from "../src/Pot.sol";

import {ERC20Mock} from "./ERC20Mock.sol";

/// MC-06: createContest / Pot constructor never validate players, rewards and totalRewards against each

/// other. remainingRewards is seeded from totalRewards independently of the rewards array, duplicates

/// overwrite, zero entries still count in i_players.length, and an empty players list makes closePot

/// divide by zero.

contract PoC_MC06 is Test {

address admin = makeAddr("admin");

address p1 = makeAddr("p1");

address p2 = makeAddr("p2");

address p3 = makeAddr("p3");

uint256 constant START = 1_000_000;

ContestManager conMan;

ERC20Mock token;

function setUp() public {

vm.startPrank(admin);

conMan = new ContestManager();

token = new ERC20Mock("TKN", "TKN", admin, 0);

token.mint(admin, START);

token.approve(address(conMan), type(uint256).max);

vm.stopPrank();

}

function _create(address[] memory players, uint256[] memory rewards, uint256 total) internal returns (Pot pot) {

vm.prank(admin);

pot = Pot(conMan.createContest(players, rewards, token, total));

}

function _createAndFund(address[] memory players, uint256[] memory rewards, uint256 total)

internal

returns (Pot pot)

{

pot = _create(players, rewards, total);

uint256 idx = conMan.getContests().length - 1;

vm.prank(admin);

conMan.fundContest(idx);

assertEq(token.balanceOf(address(pot)), total, "pot funded with totalRewards");

}

function _close(Pot pot) internal {

vm.warp(block.timestamp + 90 days + 1);

vm.prank(admin);

conMan.closeContest(address(pot));

}

function _a2(address a, address b) internal pure returns (address[] memory r) {

r = new address[](2);

r[0] = a;

r[1] = b;

}

function _u2(uint256 a, uint256 b) internal pure returns (uint256[] memory r) {

r = new uint256[](2);

r[0] = a;

r[1] = b;

}

// ---------------------------------------------------------------------------------------------

// Baseline: consistent input (sum(rewards) == totalRewards, unique players) works as intended.

// ---------------------------------------------------------------------------------------------

function test_baseline_consistentInputWorks() public {

Pot pot = _createAndFund(_a2(p1, p2), _u2(500, 500), 1000);

vm.prank(p1);

pot.claimCut();

vm.prank(p2);

pot.claimCut();

assertEq(token.balanceOf(p1), 500);

assertEq(token.balanceOf(p2), 500);

assertEq(pot.getRemainingRewards(), 0);

_close(pot);

assertEq(token.balanceOf(address(pot)), 0, "nothing left in pot");

}

// ---------------------------------------------------------------------------------------------

// Limb 1: sum(rewards) > totalRewards -> later claimer underflows (Panic 0x11) and can never claim.

// ---------------------------------------------------------------------------------------------

function test_limb1_sumAboveTotal_laterClaimerUnderflows() public {

Pot pot = _createAndFund(_a2(p1, p2), _u2(600, 600), 1000);

assertEq(pot.checkCut(p2), 600, "p2 is promised 600");

vm.prank(p1);

pot.claimCut();

assertEq(pot.getRemainingRewards(), 400);

vm.prank(p2);

vm.expectRevert(stdError.arithmeticError); // Panic(0x11)

pot.claimCut();

// A second funding does not help: remainingRewards is never re-seeded.

vm.prank(admin);

conMan.fundContest(0);

vm.prank(p2);

vm.expectRevert(stdError.arithmeticError);

pot.claimCut();

_close(pot);

console.log("p2 promised:", pot.checkCut(p2));

console.log("p2 received after close:", token.balanceOf(p2));

assertEq(token.balanceOf(p2), 0, "p2 never receives anything");

assertEq(pot.checkCut(p2), 600, "p2's promised reward still recorded but unclaimable");

}

// ---------------------------------------------------------------------------------------------

// Limb 2: sum(rewards) < totalRewards -> phantom 'unclaimed' funds feed manager cut and bonus.

// ---------------------------------------------------------------------------------------------

function test_limb2_sumBelowTotal_phantomRemainder() public {

Pot pot = _createAndFund(_a2(p1, p2), _u2(300, 300), 1000);

vm.prank(p1);

pot.claimCut();

vm.prank(p2);

pot.claimCut();

// Every player claimed in full, yet the pot reports 400 'unclaimed'.

assertEq(pot.getRemainingRewards(), 400, "phantom remainder although nobody forfeited");

uint256 mgrBefore = token.balanceOf(address(conMan));

_close(pot);

uint256 mgrCut = token.balanceOf(address(conMan)) - mgrBefore;

console.log("manager cut from phantom remainder:", mgrCut);

console.log("p1 total (300 promised):", token.balanceOf(p1));

assertEq(mgrCut, 40, "manager takes 10% of phantom remainder");

assertEq(token.balanceOf(p1), 300 + 180, "p1 gets a bonus nobody forfeited");

assertEq(token.balanceOf(p2), 300 + 180, "p2 gets a bonus nobody forfeited");

}

// ---------------------------------------------------------------------------------------------

// Limb 3: duplicate players -> earlier reward overwritten (unclaimable, counted as unclaimed) and

// the duplicate inflates the i_players.length divisor.

// ---------------------------------------------------------------------------------------------

function test_limb3_duplicatePlayers_overwriteAndDivisorInflation() public {

address[] memory players = new address[](3);

players[0] = p1;

players[1] = p1; // duplicate

players[2] = p2;

uint256[] memory rewards = new uint256[](3);

rewards[0] = 500;

rewards[1] = 200;

rewards[2] = 300;

Pot pot = _createAndFund(players, rewards, 1000);

assertEq(pot.checkCut(p1), 200, "p1's 500 entry was overwritten by the later 200 entry");

vm.prank(p1);

pot.claimCut();

vm.prank(p2);

pot.claimCut();

// Both players claimed everything they can; the overwritten 500 is now 'unclaimed'.

assertEq(pot.getRemainingRewards(), 500, "overwritten reward counted as unclaimed");

vm.prank(p1);

vm.expectRevert(Pot.Pot__RewardNotFound.selector);

pot.claimCut(); // p1 cannot claim the overwritten 500

_close(pot);

// managerCut = 50; claimantCut = 450 / 3 (duplicate inflates divisor) = 150, not 450 / 2 = 225.

console.log("p1 total (entries 500+200):", token.balanceOf(p1));

console.log("p2 total:", token.balanceOf(p2));

console.log("left stuck in pot:", token.balanceOf(address(pot)));

assertEq(token.balanceOf(p1), 200 + 150);

assertEq(token.balanceOf(p2), 300 + 150);

assertEq(token.balanceOf(address(pot)), 150, "1/3 share for the phantom duplicate slot stuck in pot");

}

// ---------------------------------------------------------------------------------------------

// Limb 4: zero-reward / zero-address entries cannot claim but inflate i_players.length,

// shrinking every claimant's bonus. Differential vs the same contest without the dead entries.

// ---------------------------------------------------------------------------------------------

function test_limb4_zeroEntriesShrinkBonus() public {

// Reference: [p1, p2] rewards [500, 400], total 900; p2 does not claim.

Pot ref = _createAndFund(_a2(p1, p2), _u2(500, 400), 900);

vm.prank(p1);

ref.claimCut();

uint256 p1Before = token.balanceOf(p1);

_close(ref);

uint256 refBonus = token.balanceOf(p1) - p1Before;

// Same contest plus a zero-reward entry (p3) and a zero-address entry.

address[] memory players = new address[](4);

players[0] = p1;

players[1] = p2;

players[2] = p3;

players[3] = address(0);

uint256[] memory rewards = new uint256[](4);

rewards[0] = 500;

rewards[1] = 400;

rewards[2] = 0;

rewards[3] = 0;

Pot pot = _createAndFund(players, rewards, 900);

vm.prank(p3);

vm.expectRevert(Pot.Pot__RewardNotFound.selector);

pot.claimCut(); // zero-reward entry cannot claim

vm.prank(p1);

pot.claimCut();

p1Before = token.balanceOf(p1);

_close(pot);

uint256 bonus = token.balanceOf(p1) - p1Before;

console.log("p1 bonus, clean list:", refBonus);

console.log("p1 bonus, with zero-reward + zero-address entries:", bonus);

// remaining 400 -> manager 40 -> 360 / 2 = 180 vs 360 / 4 = 90.

assertEq(refBonus, 180);

assertEq(bonus, 90, "dead entries halve the claimant's bonus");

}

// ---------------------------------------------------------------------------------------------

// Limb 5: empty players with totalRewards > 0 -> closePot divides by zero forever; funds locked.

// ---------------------------------------------------------------------------------------------

function test_limb5_emptyPlayers_closeDivByZero_fundsLocked() public {

Pot pot = _createAndFund(new address[](0), new uint256[](0), 1000);

vm.prank(p1);

vm.expectRevert(Pot.Pot__RewardNotFound.selector);

pot.claimCut();

vm.warp(block.timestamp + 90 days + 1);

vm.prank(admin);

vm.expectRevert(stdError.divisionError); // Panic(0x12)

conMan.closeContest(address(pot));

// Still reverts arbitrarily later.

vm.warp(block.timestamp + 3650 days);

vm.prank(admin);

vm.expectRevert(stdError.divisionError);

conMan.closeContest(address(pot));

console.log("tokens locked in pot:", token.balanceOf(address(pot)));

assertEq(token.balanceOf(address(pot)), 1000, "all funded tokens locked");

assertEq(pot.getRemainingRewards(), 1000);

}

// ---------------------------------------------------------------------------------------------

// Limb 6a: shorter rewards array -> deploy reverts (benign, index out of bounds).

// ---------------------------------------------------------------------------------------------

function test_limb6a_shorterRewardsRevertsAtDeploy() public {

uint256[] memory rewards = new uint256[](1);

rewards[0] = 500;

vm.prank(admin);

vm.expectRevert(stdError.indexOOBError); // Panic(0x32), bubbled from `new Pot`

conMan.createContest(_a2(p1, p2), rewards, token, 1000);

}

// ---------------------------------------------------------------------------------------------

// Limb 6b: longer rewards array -> extra entries silently ignored; if totalRewards includes them

// they become phantom 'unclaimed' funds.

// ---------------------------------------------------------------------------------------------

function test_limb6b_longerRewardsSilentlyIgnored() public {

address[] memory players = new address[](1);

players[0] = p1;

Pot pot = _createAndFund(players, _u2(500, 500), 1000); // deploys fine

assertEq(pot.checkCut(p1), 500);

vm.prank(p1);

pot.claimCut();

assertEq(pot.getRemainingRewards(), 500, "second reward entry silently ignored, counted as unclaimed");

_close(pot);

console.log("p1 total (500 promised):", token.balanceOf(p1));

console.log("manager cut:", token.balanceOf(address(conMan)));

assertEq(token.balanceOf(address(conMan)), 50);

assertEq(token.balanceOf(p1), 500 + 450);

}

Recommended Mitigation

Validate the parameters once, in the Pot constructor, so every deployment path is covered. The duplicate check reuses playersToRewards, which keeps the cost O(n).

--- a/src/Pot.sol

+++ b/src/Pot.sol

@@ -8,6 +8,7 @@

error Pot__RewardNotFound();

error Pot__InsufficientFunds();

error Pot__StillOpenForClaim();

+ error Pot__InvalidContestParameters();

address[] private i_players;

uint256[] private i_rewards;

@@ -20,6 +21,9 @@

uint256 private constant managerCutPercent = 10;

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {

+ if (players.length == 0 || players.length != rewards.length) {

+ revert Pot__InvalidContestParameters();

+ }

i_players = players;

i_rewards = rewards;

i_token = token;

@@ -29,8 +33,17 @@

// i_token.transfer(address(this), i_totalRewards);

+ uint256 sum;

for (uint256 i = 0; i < i_players.length; i++) {

+ // rejects zero addresses, zero rewards and duplicate players

+ if (i_players[i] == address(0) || i_rewards[i] == 0 || playersToRewards[i_players[i]] != 0) {

+ revert Pot__InvalidContestParameters();

+ }

playersToRewards[i_players[i]] = i_rewards[i];

+ sum += i_rewards[i];

+ }

+ if (sum != totalRewards) {

+ revert Pot__InvalidContestParameters();

}

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago

Submission Judgement Published

Invalidated

Reason: Incorrect statement

Rewards breakdown

This contest has a flat reward structure with the following payouts:

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being reviewed by our AI judge. Results will be available in a few minutes.

View all submissions

Rewards Distribution

The contest is complete and the rewards are being distributed.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!

MyCut

createContest and the Pot constructor never validate players, rewards and totalRewards (sum mismatch, duplicates, zero entries, empty list), so promised rewards become unclaimable, phantom remainders are paid out or the pot cannot close

Missing input validation in createContest / Pot constructor + denied claims, misallocated bonus or a pot that can never close

Description

Risk

Proof of Concept

Recommended Mitigation

Lead Judging Commences

Rewards breakdown

Live

Judging

Rewards Distribution

FAQs

Missing input validation in `createContest` / `Pot` constructor + denied claims, misallocated bonus or a pot that can never close