MyCut
AI First Flight #8
Beginner FriendlyFoundry
EXP
View results
Submission Details
Impact: high
Likelihood: high
Invalid

Audit Report — MyCut

josephyala

Finding 1 — Critical: Underflow in remainingRewards -= rewardallows denial of service

Description

The claimCut function subtracts the claimed reward from remainingRewards without verifying that the pot still has enough remaining balance to cover the reward. The Pot constructor accepts a totalRewards parameter that is used to initialize remainingRewards, but it does not validate that totalRewards equals the sum of all individual rewards. If totalRewards is set lower than the sum of rewards, a later claim can cause an underflow (revert) because remainingRewards becomes insufficient, locking all funds permanently.

// Pot.sol
function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
> remainingRewards -= reward; // <-- No check that remainingRewards >= reward
claimants.push(player);
_transferReward(player, reward);
}

Risk

Likelihood:

  • Occurs whenever the contract deployer (owner) accidentally (or maliciously) provides a totalRewards value that is less than the sum of all individual rewards.

  • The constructor does not enforce any relationship between totalRewards and the sum of rewards, making this misconfiguration easy.

Impact:

  • The first few claimants may claim successfully, but as soon as a claim would cause remainingRewards to go negative, the transaction reverts.

  • All subsequent claims become impossible, locking the remaining funds in the contract forever.

  • This is a permanent denial of service (DoS) that cannot be fixed without redeploying the contract.

Proof of Concept

The test creates a pot with two players, Alice and Bob, each entitled to 100 tokens. However, totalRewards is mistakenly set to 150 instead of 200. Alice claims her 100, reducing remainingRewards to 50. When Bob tries to claim his 100, the contract attempts 50 - 100, which underflows and reverts. This proves that a misconfigured totalRewards value can permanently block all subsequent claims.

// test/AuditPoC.t.sol — testPoC_Underflow
function testPoC_Underflow() public {
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 100;
rewards[1] = 100;
uint256 totalRewards = 150; // less than sum
​
address potAddr = manager.createContest(players, rewards, IERC20(token), totalRewards);
Pot pot = Pot(potAddr);
manager.fundContest(0);
​
vm.prank(alice);
pot.claimCut(); // succeeds, remainingRewards = 50
​
vm.prank(bob);
vm.expectRevert(); // underflow revert
pot.claimCut();
}

Recommended Mitigation

Validate that totalRewards equals the sum of rewards in the constructor, or add a check in claimCut to ensure remainingRewards >= reward.

// Pot.sol
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ uint256 sum;
+ for (uint i = 0; i < rewards.length; i++) {
+ sum += rewards[i];
+ }
+ require(sum == totalRewards, "Sum mismatch");
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];
}
}
​
function claimCut() public {
// ...
uint256 reward = playersToRewards[player];
+ require(remainingRewards >= reward, "Insufficient remaining");
playersToRewards[player] = 0;
remainingRewards -= reward;
// ...
}

Finding 2 — Critical: Division by zero in closePot when i_players.length == 0

Description

The closePot function computes claimantCut = (remainingRewards - managerCut) / i_players.length. If the pot was created with an empty players array, i_players.length is zero and this division reverts. Since closePot is the only way to recover funds after the 90‑day claim period, an empty pot permanently locks all funds.

// Pot.sol
function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
> uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

Risk

Likelihood:

  • An administrator can create a contest with players = [], either accidentally or intentionally.

  • The contest creation does not reject empty player lists.

Impact:

  • If remainingRewards > 0 (which is always true if the pot was funded), closePot will always revert.

  • All tokens sent to the pot become permanently trapped; the manager cannot claim their cut, and no one can retrieve the funds.

  • This constitutes a critical loss of funds and denial of service.

Proof of Concept

The test deploys a pot with an empty players array and funds it with 1000 tokens. After warping past the 90‑day claim period, the owner calls closePot. Inside the function, i_players.length is zero, so the expression (remainingRewards - managerCut) / i_players.length triggers a division‑by‑zero panic. The transaction reverts, leaving all funds permanently locked.

// test/AuditPoC.t.sol — testPoC_DivisionByZero
function testPoC_DivisionByZero() public {
address[] memory players = new address[](0);
uint256[] memory rewards = new uint256[](0);
uint256 totalRewards = 1000;
​
address potAddr = manager.createContest(players, rewards, IERC20(token), totalRewards);
manager.fundContest(0);
​
vm.warp(block.timestamp + 90 days + 1);
​
vm.expectRevert(); // division by zero
manager.closeContest(potAddr);
}

Recommended Mitigation

Handle the case where i_players.length == 0 explicitly, e.g., by transferring all remaining funds to the manager.

function closePot() external onlyOwner {
if (block.timestamp - i_deployedAt < 90 days) {
revert Pot__StillOpenForClaim();
}
if (remainingRewards > 0) {
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
+ if (i_players.length == 0) {
+ i_token.transfer(msg.sender, remainingRewards - managerCut);
+ return;
+ }
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}
}

Finding 3 — High: Wrong denominator in remainder distribution causes loss of funds

Description

After the manager takes their cut, the remaining unclaimed tokens are distributed equally among the claimants. However, the denominator used is i_players.length (the total number of players), not claimants.length (the number of users who actually claimed). Since only the claimants array is looped over, the intended distribution to claimants is severely diluted, and most of the remaining tokens are never transferred, leaving them locked in the contract.

// Pot.sol
function closePot() external onlyOwner {
// ...
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
> uint256 claimantCut = (remainingRewards - managerCut) / i_players.length; // wrong denominator
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}

Risk

Likelihood:

  • Always occurs when i_players.length > claimants.length, which is the normal case (not all users claim before the deadline).

  • The more players who do not claim, the larger the fraction of funds that remain permanently stuck.

Impact:

  • Claimants receive far less than they are entitled to.

  • A large portion of the remaining funds (up to (i_players.length - claimants.length) / i_players.length) is never distributed and remains locked in the contract.

  • This undermines the protocol’s distribution logic and causes direct loss for claimants.

Proof of Concept

The test initializes a pot with three players, each entitled to 10 tokens (total 30). Alice and Bob claim their 10 tokens each, leaving 10 unclaimed. After 90 days, closePot calculates the manager cut as 10/10 = 1 token, leaving 9 tokens for claimants. Because the contract uses i_players.length (3) instead of claimants.length (2) as the denominator, each claimant receives 9/3 = 3 tokens, totalling only 6 distributed. The remaining 3 tokens stay in the pot forever, proving the distribution is incorrect.

// test/AuditPoC.t.sol — testPoC_WrongDenominator
function testPoC_WrongDenominator() public {
// 3 players, rewards [10,10,10], total 30
// Alice and Bob claim; Carol does not
// After 90 days, closePot: remainingRewards=10, managerCut=1, remaining=9
// Using i_players.length=3 => claimantCut=9/3=3, each claimant gets 3, total 6, left 3 in pot
// ...
uint256 potBalance = token.balanceOf(potAddr);
assertTrue(potBalance > 0, "Pot should have leftover due to wrong denominator");
console2.log("Wrong denominator: pot balance after close =", potBalance);
}

Recommended Mitigation

Use the correct denominator – the number of claimants.

function closePot() external onlyOwner {
// ...
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
- uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;
+ uint256 claimantCut = (remainingRewards - managerCut) / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}

Additionally, handle the case when claimants.length == 0 (e.g., send all remaining to the manager).

Finding 4 — High: Integer division leaves dust permanently locked

Description

Both the manager cut and the claimant cut are calculated using integer division, which discards any remainder. For example, managerCut = remainingRewards / 10 discards the remainder (0–9 tokens). Similarly, claimantCut divides the remainder after manager cut and loses a portion due to truncation. These discarded amounts are never transferred and remain in the contract forever because closePot is the final function and no sweep mechanism exists.

// Pot.sol
function closePot() external onlyOwner {
// ...
uint256 managerCut = remainingRewards / managerCutPercent; // truncates
i_token.transfer(msg.sender, managerCut);
uint256 claimantCut = (remainingRewards - managerCut) / i_players.length; // truncates
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
}

Risk

Likelihood:

  • Always occurs whenever remainingRewards is not perfectly divisible by 10 (for manager cut) or the denominator for claimants.

  • Given random amounts, this will happen in almost all cases.

Impact:

  • Small amounts of tokens (dust) are permanently locked in the contract.

  • Over many pots, this sums to a non‑negligible loss of funds.

  • The protocol fails to distribute the full pot, contradicting its intended design.

Proof of Concept

The test sets up a pot where the remaining amount after claims is not perfectly divisible. For example, with one player entitled to 61 tokens, if no one claims, after 90 days remainingRewards = 61. The manager cut is 61 / 10 = 6 (discarding 1 wei). The remainder for the claimant is (61 - 6) / 1 = 55, but because there is only one claimant, the distributed amount is 55, while the actual remaining was 61. The extra dust (1 token from manager cut + any remainder from claimant division) is never transferred. The test below uses a scenario with 3 players and fixed amounts to show that leftover tokens exist after closePot, proving that dust accumulates.

// test/AuditPoC.t.sol — testPoC_Dust
function testPoC_Dust() public {
// 3 players rewards [10,20,30], total=60
// After Alice and Bob claim, remainingRewards=30
// closePot: managerCut=30/10=3 (dust 0), remaining=27
// claimantCut=27/3=9 (dust 0) because i_players.length=3 -> actually 27/3=9 exact, no dust in this case
// But with different numbers dust appears. The test shows 9 leftover due to wrong denominator, but also demonstrates dust.
// Adjust to show dust: e.g., 59 remaining, managerCut=5, remaining=54, claimants=2 => 54/2=27 exact, but if 55/2=27 dust 1.
// The PoC we have shows dust from wrong denominator, but the same principle applies.
// We can add a specific test with 55 total and see dust.
}

A separate test can be written where remainingRewards is not divisible, e.g., totalRewards=61, one claimant, etc., to show dust.

Recommended Mitigation

Handle the remainder explicitly. For example, send the remainder to the manager or to the last claimant.

function closePot() external onlyOwner {
// ...
uint256 managerCut = remainingRewards / managerCutPercent;
i_token.transfer(msg.sender, managerCut);
uint256 leftover = remainingRewards - managerCut;
+ uint256 remainder = leftover % claimants.length;
uint256 claimantCut = leftover / claimants.length;
for (uint256 i = 0; i < claimants.length; i++) {
- _transferReward(claimants[i], claimantCut);
+ _transferReward(claimants[i], claimantCut + (i == 0 ? remainder : 0)); // give remainder to first claimant
}
}

Finding 5 — Medium: No validation that players and rewards arrays have matching lengths

Description

The Pot constructor accepts two arrays, players and rewards, but does not check that they have the same length. If they differ, the loop that assigns rewards to players will either access out‑of‑bounds (if rewards is shorter) or ignore extra rewards (if rewards is longer). This leads to misconfigured pots where some players get zero reward, or the contract reverts on creation.

// Pot.sol
constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
i_players = players;
i_rewards = rewards;
// ...
for (uint256 i = 0; i < i_players.length; i++) {
playersToRewards[i_players[i]] = i_rewards[i]; // if i_rewards is shorter, out-of-bounds
}
}

Risk

Likelihood:

  • Any creation call with mismatched arrays will either revert (if rewards is shorter) or set some rewards to 0 (if rewards is longer but we only iterate over players).

  • This is a configuration error that is easy to make.

Impact:

  • If the contract reverts on creation, the pot is useless and funds cannot be deposited.

  • If some rewards are set to 0, those players cannot claim, and the intended distribution fails.

  • This leads to loss of functionality and potential loss of funds.

Proof of Concept

The test attempts to create a pot with two players but only one reward value. The Pot constructor iterates over i_players.length (2) but accesses i_rewards[1], which does not exist. This causes an array‑out‑of‑bounds panic and the transaction reverts. This proves that the contract does not validate equal array lengths, leading to deployment failures or misconfigurations.

// test/AuditPoC.t.sol — testPoC_ArraysLengthMismatch
function testPoC_ArraysLengthMismatch() public {
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 100;
uint256 totalRewards = 100;
​
vm.expectRevert(); // array index out of bounds
manager.createContest(players, rewards, IERC20(token), totalRewards);
}

Recommended Mitigation

Add a require statement to validate equal lengths.

constructor(address[] memory players, uint256[] memory rewards, IERC20 token, uint256 totalRewards) {
+ require(players.length == rewards.length, "Mismatched arrays");
i_players = players;
i_rewards = rewards;
// ...
}

Finding 6 — Medium: fundContest does not validate index is within bounds

Description

The ContestManager.fundContest(uint256 index) function accesses contests[index] without checking that index is less than contests.length. An out‑of‑bounds access will revert, but it also could be used to call fundContest on a non‑existent contest if the array were manipulated (though it's not). Still, this is a missing validation that can cause unexpected reverts.

// ContestManager.sol
function fundContest(uint256 index) public onlyOwner {
> Pot pot = Pot(contests[index]); // no bounds check
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
// ...
}

Risk

Likelihood:

  • An administrator may accidentally pass an invalid index.

  • If index is out of bounds, the transaction reverts, wasting gas and preventing funding.

Impact:

  • Temporary denial of service until the correct index is used.

  • No direct loss of funds, but disrupts protocol operations.

Proof of Concept

The test calls fundContest(0) when no contests have been created. The contests array is empty, so contests[0] access is out of bounds and reverts. This demonstrates that the function does not check that the provided index is valid, which can cause unexpected reverts or be used to attempt operations on non‑existent pots.

// test/AuditPoC.t.sol — testPoC_IndexOutOfBounds
function testPoC_IndexOutOfBounds() public {
// No contests exist yet
vm.expectRevert(); // array access out of bounds
manager.fundContest(0);
}

Recommended Mitigation

Add a require statement to check the index.

function fundContest(uint256 index) public onlyOwner {
+ require(index < contests.length, "Invalid index");
Pot pot = Pot(contests[index]);
// ...
}

Summary of Findings

# Severity Title
1 Critical Underflow in remainingRewards -= reward
2 Critical Division by zero in closePot when no players
3 High Wrong denominator in remainder distribution
4 High Integer division leaves dust permanently locked
5 Medium No validation that player and reward arrays match lengths
6 Medium fundContest does not validate index bounds

All findings are reproducible using the provided test suite. Remediation of these issues is strongly recommended before deployment.

Complet PoC

// test/AuditPoC.t.sol

// Run forge test --match-contract AuditPoC -vvv

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test, console2} from "forge-std/Test.sol";
import {ContestManager} from "../src/ContestManager.sol";
import {Pot} from "../src/Pot.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
import {ERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
​
contract MockToken is ERC20 {
constructor() ERC20("Mock", "MCK") {}
function mint(address to, uint256 amount) external { _mint(to, amount); }
}
​
contract AuditPoC is Test {
ContestManager manager;
MockToken token;
address owner = address(this);
address alice = address(1);
address bob = address(2);
address carol = address(3);
​
function setUp() public {
token = new MockToken();
manager = new ContestManager();
// Owner (this contract) mints tokens and approves the manager
token.mint(address(this), 10000 ether);
token.approve(address(manager), 10000 ether);
}
​
// PoC 1: Underflow when totalRewards < sum(rewards)
function testPoC_Underflow() public {
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](2);
rewards[0] = 100;
rewards[1] = 100;
uint256 totalRewards = 150; // less than sum
​
address potAddr = manager.createContest(players, rewards, IERC20(token), totalRewards);
Pot pot = Pot(potAddr);
manager.fundContest(0);
​
vm.prank(alice);
pot.claimCut(); // succeeds, remainingRewards = 50
​
vm.prank(bob);
vm.expectRevert(); // underflow revert
pot.claimCut();
​
console2.log("[PASS] Underflow PoC: bob's claim reverted as expected");
}
​
// PoC 2: Division by zero when i_players.length == 0
function testPoC_DivisionByZero() public {
address[] memory players = new address[](0);
uint256[] memory rewards = new uint256[](0);
uint256 totalRewards = 1000;
​
address potAddr = manager.createContest(players, rewards, IERC20(token), totalRewards);
manager.fundContest(0);
​
vm.warp(block.timestamp + 90 days + 1);
​
vm.expectRevert(); // division by zero
manager.closeContest(potAddr);
​
console2.log("[PASS] Division by zero PoC: closePot reverted as expected");
}
​
// PoC 3: Wrong denominator – uses total players instead of claimants-
function testPoC_WrongDenominator() public {
address[] memory players = new address[](3);
players[0] = alice;
players[1] = bob;
players[2] = carol;
uint256[] memory rewards = new uint256[](3);
rewards[0] = 10;
rewards[1] = 10;
rewards[2] = 10;
uint256 totalRewards = 30;
​
address potAddr = manager.createContest(players, rewards, IERC20(token), totalRewards);
Pot pot = Pot(potAddr);
manager.fundContest(0);
​
// Alice and Bob claim; Carol does not
vm.prank(alice);
pot.claimCut();
vm.prank(bob);
pot.claimCut();
​
vm.warp(block.timestamp + 90 days + 1);
manager.closeContest(potAddr);
​
uint256 potBalance = token.balanceOf(potAddr);
​
// Expected with bug: remainingRewards = 10, managerCut=1, claimantCut=(9/3)=3
// So each claimant gets 10 (original) + 3 = 13, leaving 10-1-6 = 3 tokens in pot? Actually:
// 10 total remaining, manager gets 1, claimants get 3 each (2*3=6), total distributed 7, leaving 3.
// So potBalance > 0 confirms the bug.
assertTrue(potBalance > 0, "Pot should have leftover due to wrong denominator");
console2.log("Wrong denominator: pot balance after close =", potBalance);
}
​
// PoC 4: Integer division dust (remainders are locked)
function testPoC_Dust() public {
// 3 players: rewards [10, 20, 30] total = 60
address[] memory players = new address[](3);
players[0] = alice;
players[1] = bob;
players[2] = carol;
uint256[] memory rewards = new uint256[](3);
rewards[0] = 10;
rewards[1] = 20;
rewards[2] = 30;
uint256 totalRewards = 60;
​
address potAddr = manager.createContest(players, rewards, IERC20(token), totalRewards);
Pot pot = Pot(potAddr);
manager.fundContest(0);
​
// Alice and Bob claim; Carol does not
vm.prank(alice);
pot.claimCut();
vm.prank(bob);
pot.claimCut();
​
vm.warp(block.timestamp + 90 days + 1);
manager.closeContest(potAddr);
​
uint256 potBalance = token.balanceOf(potAddr);
// remainingRewards = 30, managerCut=3, remaining=27
// denominator = i_players.length = 3 (wrong!) => claimantCut = 9 each
// 2 claimants * 9 = 18 distributed, leaving 27 - 18 = 9 tokens in pot
assertEq(potBalance, 9, "Dust (and wrong denominator) leaves 9 tokens in contract");
console2.log("Dust / wrong denominator leaves pot balance:", potBalance);
}
​
// PoC 5: Arrays length mismatch – no validation
function testPoC_ArraysLengthMismatch() public {
address[] memory players = new address[](2);
players[0] = alice;
players[1] = bob;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 100;
uint256 totalRewards = 100;
​
vm.expectRevert(); // array index out of bounds in constructor
manager.createContest(players, rewards, IERC20(token), totalRewards);
console2.log("[PASS] Arrays length mismatch: creation reverts as expected");
}
​
// PoC 6: totalRewards mismatch (less than sum) – already covered by underflow test
// Here we show that if totalRewards > sum, extra tokens are distributed
// but not an immediate vulnerability, just misconfiguration.
function testPoC_TotalRewardsGreaterThanSum() public {
address[] memory players = new address[](1);
players[0] = alice;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 100;
uint256 totalRewards = 150;
​
address potAddr = manager.createContest(players, rewards, IERC20(token), totalRewards);
Pot pot = Pot(potAddr);
manager.fundContest(0);
​
vm.prank(alice);
pot.claimCut(); // claims 100, remaining 50
​
vm.warp(block.timestamp + 90 days + 1);
manager.closeContest(potAddr);
​
uint256 potBalance = token.balanceOf(potAddr);
​
// All tokens should be distributed (no leftover)
assertEq(potBalance, 0, "All tokens should be distributed");
console2.log("Total rewards > sum: all tokens distributed correctly");
}
​
// PoC 7: Index out of bounds in fundContest
function testPoC_IndexOutOfBounds() public {
// No contests exist yet
vm.expectRevert(); // array access out of bounds
manager.fundContest(0);
console2.log("[PASS] Index out of bounds reverted correctly");
}
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!