MyCut

AI First Flight #8

Beginner FriendlyFoundry

EXP

View results

Previous Next

Submission Details

Impact: medium

Likelihood: low

Invalid

Pot and ContestManager ignore the bool returned by ERC20 transfer/transferFrom; with an EIP-20 token that returns false, claimCut erases a player's entitlement without paying them and fundContest succeeds while funding nothing

friedsocks233

Unchecked ERC20 return values + entitlement erased without payment and silent non-funding

Description

Every token movement should either succeed or revert. A successful claimCut should mean the player was paid, and a successful fundContest should mean the Pot holds totalRewards. EIP-20 lets a compliant token signal failure by returning false, and says callers must handle that.
Every token call ignores the return value: transfer at Pot.sol:55 and Pot.sol:65, and transferFrom at ContestManager.sol:37.
claimCut applies all its effects first: it zeroes the entitlement, decrements remainingRewards and pushes the player to claimants. Only then does it make the unchecked transfer. With a token that returns false instead of reverting, a failed payment is recorded as a successful one.
fundContest checks only balanceOf, never the allowance, so a transferFrom that returns false also passes unnoticed.

// src/Pot.sol

43: playersToRewards[player] = 0;

44: remainingRewards -= reward;

45: claimants.push(player);

46: _transferReward(player, reward);

// ...

@> 55: i_token.transfer(msg.sender, managerCut);

// ...

64: function _transferReward(address player, uint256 reward) internal {

@> 65: i_token.transfer(player, reward);

66: }

// src/ContestManager.sol

33: if (token.balanceOf(msg.sender) < totalRewards) {

34: revert ContestManager__InsufficientFunds();

35: }

36:

@> 37: token.transferFrom(msg.sender, address(pot), totalRewards);

Risk

Likelihood:

It requires a pot created with a token that returns false on failure. EIP-20 permits such tokens, but the in-repo OpenZeppelin 5.0.2 token reverts instead, and the README limits scope to "Standard ERC20 Tokens Only".
It also requires a pot that is unfunded when a player claims. That happens when a player claims in the gap between the separate createContest and fundContest transactions, or when fundContest itself silently failed because the owner had not approved enough.

Impact:

A player who claims against an unfunded pot permanently loses their whole reward. In the PoC, p1's claim succeeds, p1 receives 0 and checkCut(p1) becomes 0. After the pot is funded, p1's second claim reverts with Pot__RewardNotFound, and p1's 400 sits in the Pot while remainingRewards reads 0.
fundContest reports success while funding nothing: getContestRemainingRewards still says 1000 with a 0 balance. Every player who then claims is wiped without payment, and closePot's transfers also fail silently on such a pot.

Proof of Concept

The PoC defines a minimal EIP-20 token inline (FalseReturnToken) that returns false on insufficient balance or allowance instead of reverting. All four tests pass:

test_baseline_OZToken_revertsAndPreservesEntitlement (control): with the repo's ERC20Mock, the same mistakes revert (ERC20InsufficientBalance, ERC20InsufficientAllowance), and the player's entitlement is kept.
test_limb1_claimCutErasesEntitlementWithoutPayment: a claim on an unfunded pot succeeds and pays 0. The entitlement is erased, and after funding p1's 400 is stranded while p1 cannot claim again.
test_limb2_fundContestSucceedsWhileFundingNothing: without an approval, fundContest succeeds, the Pot balance stays 0, and both players' claims wipe their entitlements without paying them.
test_limb3_closePotTransfersFailSilently: closeContest on that pot succeeds and pays neither the manager cut nor the bonus.

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

FOUNDRY_FFI=false forge test --match-path test/PoC_MC-07.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";

/// Minimal EIP-20 token that signals failure by returning false (allowed by EIP-20),

/// instead of reverting. Balances/allowances are only changed on success.

contract FalseReturnToken {

string public name = "FalseReturn";

string public symbol = "FRT";

uint8 public decimals = 18;

uint256 public totalSupply;

mapping(address => uint256) public balanceOf;

mapping(address => mapping(address => uint256)) public allowance;

event Transfer(address indexed from, address indexed to, uint256 value);

event Approval(address indexed owner, address indexed spender, uint256 value);

function mint(address to, uint256 amount) external {

balanceOf[to] += amount;

totalSupply += amount;

emit Transfer(address(0), to, amount);

}

function approve(address spender, uint256 amount) external returns (bool) {

allowance[msg.sender][spender] = amount;

emit Approval(msg.sender, spender, amount);

return true;

}

function transfer(address to, uint256 amount) external returns (bool) {

if (balanceOf[msg.sender] < amount) return false;

balanceOf[msg.sender] -= amount;

balanceOf[to] += amount;

emit Transfer(msg.sender, to, amount);

return true;

}

function transferFrom(address from, address to, uint256 amount) external returns (bool) {

if (balanceOf[from] < amount || allowance[from][msg.sender] < amount) return false;

allowance[from][msg.sender] -= amount;

balanceOf[from] -= amount;

balanceOf[to] += amount;

emit Transfer(from, to, amount);

return true;

}

/// MC-07: transfer / transferFrom return values are ignored (Pot.sol:55, :65; ContestManager.sol:37).

contract PoC_MC07 is Test {

address admin = makeAddr("admin");

address p1 = makeAddr("p1");

address p2 = makeAddr("p2");

ContestManager conMan;

address[] players;

uint256[] rewards;

uint256 constant TOTAL = 1_000;

function setUp() public {

vm.prank(admin);

conMan = new ContestManager();

players.push(p1);

players.push(p2);

rewards.push(400);

rewards.push(600);

}

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

// Baseline: with the in-repo OZ 5.0.2 token, the same mistakes revert,

// so the player's entitlement is preserved.

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

function test_baseline_OZToken_revertsAndPreservesEntitlement() public {

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

oz.mint(admin, TOTAL);

vm.prank(admin);

Pot pot = Pot(conMan.createContest(players, rewards, IERC20(address(oz)), TOTAL));

// claim before funding: reverts, entitlement intact

vm.prank(p1);

vm.expectRevert(

abi.encodeWithSelector(IERC20Errors.ERC20InsufficientBalance.selector, address(pot), 0, 400)

);

pot.claimCut();

assertEq(pot.checkCut(p1), 400, "baseline: entitlement preserved");

assertEq(pot.getRemainingRewards(), TOTAL, "baseline: remainingRewards unchanged");

// fundContest without approval: reverts, so admin knows funding failed

vm.prank(admin);

vm.expectRevert(

abi.encodeWithSelector(IERC20Errors.ERC20InsufficientAllowance.selector, address(conMan), 0, TOTAL)

);

conMan.fundContest(0);

}

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

// Limb 1: claimCut on an unfunded pot with a false-returning token.

// The tx succeeds, the entitlement is zeroed, nothing is paid, and after

// funding the player cannot claim again: their reward is stranded.

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

function test_limb1_claimCutErasesEntitlementWithoutPayment() public {

FalseReturnToken tok = new FalseReturnToken();

tok.mint(admin, TOTAL);

vm.prank(admin);

Pot pot = Pot(conMan.createContest(players, rewards, IERC20(address(tok)), TOTAL));

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

// p1 claims in the gap between createContest and fundContest

vm.prank(p1);

pot.claimCut(); // does NOT revert

console.log("p1 token balance after claim:", tok.balanceOf(p1));

console.log("p1 checkCut after claim:", pot.checkCut(p1));

console.log("pot remainingRewards after claim:", pot.getRemainingRewards());

assertEq(tok.balanceOf(p1), 0, "p1 was paid nothing");

assertEq(pot.checkCut(p1), 0, "p1 entitlement erased");

assertEq(pot.getRemainingRewards(), TOTAL - 400, "remainingRewards decremented as if paid");

// admin now funds the pot correctly

vm.startPrank(admin);

tok.approve(address(conMan), TOTAL);

conMan.fundContest(0);

vm.stopPrank();

assertEq(tok.balanceOf(address(pot)), TOTAL, "pot fully funded");

// p1 can never claim again

vm.prank(p1);

vm.expectRevert(Pot.Pot__RewardNotFound.selector);

pot.claimCut();

// p2 is paid normally, showing p1's 400 sits unaccounted in the pot

vm.prank(p2);

pot.claimCut();

assertEq(tok.balanceOf(p2), 600);

assertEq(pot.getRemainingRewards(), 0, "accounting says nothing is left");

assertEq(tok.balanceOf(address(pot)), 400, "p1's 400 stranded in pot");

console.log("pot balance with remainingRewards == 0:", tok.balanceOf(address(pot)));

}

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

// Limb 2: fundContest with insufficient allowance. Only balanceOf is

// checked (ContestManager.sol:33); transferFrom returns false, tx succeeds,

// pot stays empty. Players who then claim lose their entitlement.

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

function test_limb2_fundContestSucceedsWhileFundingNothing() public {

FalseReturnToken tok = new FalseReturnToken();

tok.mint(admin, TOTAL);

vm.prank(admin);

Pot pot = Pot(conMan.createContest(players, rewards, IERC20(address(tok)), TOTAL));

// admin forgot to approve (allowance 0) but has the balance

vm.prank(admin);

conMan.fundContest(0); // does NOT revert

console.log("pot balance after 'successful' fundContest:", tok.balanceOf(address(pot)));

assertEq(tok.balanceOf(address(pot)), 0, "pot funded with nothing");

assertEq(tok.balanceOf(admin), TOTAL, "admin still holds all tokens");

assertEq(conMan.getContestRemainingRewards(address(pot)), TOTAL, "accounting claims full pot");

// both players claim believing the pot funded; both are wiped, neither paid

vm.prank(p1);

pot.claimCut();

vm.prank(p2);

pot.claimCut();

assertEq(tok.balanceOf(p1), 0);

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

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

assertEq(pot.checkCut(p2), 0);

assertEq(pot.getRemainingRewards(), 0, "pot reports everything paid out");

}

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

// Limb 3: closePot transfers also fail silently. With the pot unfunded

// (limb 2), closeContest succeeds and pays neither the manager cut nor

// the claimant bonus.

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

function test_limb3_closePotTransfersFailSilently() public {

FalseReturnToken tok = new FalseReturnToken();

tok.mint(admin, TOTAL);

vm.prank(admin);

Pot pot = Pot(conMan.createContest(players, rewards, IERC20(address(tok)), TOTAL));

vm.prank(admin);

conMan.fundContest(0); // silently funds nothing (no approval)

// p1 claims (silently unpaid); p2 never claims

vm.prank(p1);

pot.claimCut();

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

vm.prank(admin);

conMan.closeContest(address(pot)); // does NOT revert

// managerCut = 600/10 = 60 to conMan; claimantCut = 540/2 = 270 to p1 -> neither paid

assertEq(tok.balanceOf(address(conMan)), 0, "manager cut not paid");

assertEq(tok.balanceOf(p1), 0, "claimant bonus not paid");

console.log("conMan balance after close:", tok.balanceOf(address(conMan)));

console.log("p1 balance after close:", tok.balanceOf(p1));

}

Recommended Mitigation

Use OpenZeppelin SafeERC20 (already in lib/, version 5.0.2) for every token movement.

--- a/src/Pot.sol

+++ b/src/Pot.sol

@@ -2,9 +2,12 @@

pragma solidity ^0.8.20;

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

+import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";

import {Ownable} from "lib/openzeppelin-contracts/contracts/access/Ownable.sol";

contract Pot is Ownable(msg.sender) {

+ using SafeERC20 for IERC20;

error Pot__RewardNotFound();

error Pot__InsufficientFunds();

error Pot__StillOpenForClaim();

@@ -52,7 +55,7 @@

}

if (remainingRewards > 0) {

uint256 managerCut = remainingRewards / managerCutPercent;

- i_token.transfer(msg.sender, managerCut);

+ i_token.safeTransfer(msg.sender, managerCut);

uint256 claimantCut = (remainingRewards - managerCut) / i_players.length;

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

@@ -62,7 +65,7 @@

}

function _transferReward(address player, uint256 reward) internal {

- i_token.transfer(player, reward);

+ i_token.safeTransfer(player, reward);

}

function getToken() public view returns (IERC20) {

--- a/src/ContestManager.sol

+++ b/src/ContestManager.sol

@@ -4,8 +4,11 @@

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

import {Ownable} from "lib/openzeppelin-contracts/contracts/access/Ownable.sol";

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

+import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";

contract ContestManager is Ownable {

+ using SafeERC20 for IERC20;

address[] public contests;

mapping(address => uint256) public contestToTotalRewards;

@@ -34,7 +37,7 @@

revert ContestManager__InsufficientFunds();

}

- token.transferFrom(msg.sender, address(pot), totalRewards);

+ token.safeTransferFrom(msg.sender, address(pot), totalRewards);

}

function getContests() public view returns (address[] memory) {

Updates

Lead Judging Commences

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