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

Unhandled Return Value in ERC20

joshuapaul1

Root + Impact

The ContestManager contract fails to track whether a contest has already been funded and ignores the return value of transferFrom. Consequently, an owner can accidentally call fundContest multiple times for the same contest, and if an ERC20 token returns false instead of reverting on failure (such as when a prior approve call is missing), the contract will statefully treat the contest as active despite receiving no funding.

Description

The contract is designed to allow the owner to deploy individual Pot contracts for contests and subsequently fund them with the required ERC20 tokens using the fundContest function.

However, fundContest lacks state validation to check if a specific contest index has already been processed, allowing the transfer logic to be executed repeatedly for the same Pot. Furthermore, the function executes transferFrom without verifying the boolean return value required by the ERC20 standard, meaning failures in standard token transfers pass unnoticed by the execution flow.

function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
//may fund the constst more than once so there is no
//keeping track of which pots have yet to receive funds or
//which ones have already received funds
​
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
​
@> token.transferFrom(msg.sender, address(pot), totalRewards);
/**
trnasfer is made before approve is made which can result in a failed txn
and the bool is ignored
*/
}
​

Risk

Likelihood: Medium

  • The owner executes fundContest multiple times due to operational error, network lag, or script retries.

  • The contract interacts with standard ERC20 tokens that return false on failed transfers rather than reverting, combined with the owner omitting the necessary prerequisite approve transaction.

Impact: High

  • User funds are double-spent or locked redundantly within a single Pot contract, draining the owner's balance.

  • The contest enters a state where it is considered functionally valid by the manager while holding a zero token balance, leading to failed prize distributions for players.

Proof of Concept

  1. The owner calls createContest to initialize a new contest requiring 1,000 tokens.

  2. The owner calls fundContest(0) twice due to a duplicate transaction submission.

  3. The owner's balance decreases by 2,000 tokens while the Pot receives double its required allocation.

  4. Alternatively, if the owner calls fundContest(0) without executing token.approve(ContestManager, 1000) beforehand on a non-reverting ERC20 token, transferFrom returns false, no tokens are moved, but the transaction completes successfully.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Test, console} from "forge-std/Test.sol";
import {ContestManager} from "../src/ContestManager.sol";
import {Pot} from "../src/Pot.sol";
import {ERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/ERC20.sol";
​
// A mock ERC20 token that returns false on failure instead of reverting
contract MockERC20isNonReverting is ERC20 {
constructor() ERC20("MockToken", "MTK") {
_mint(msg.sender, 100_000 * 10**18);
}
​
function transferFrom(address from, address to, uint256 amount) public override returns (bool) {
// If allowance is insufficient, return false instead of reverting
if (allowance(from, msg.sender) < amount && from != msg.sender) {
return false;
}
// If balance is insufficient, return false instead of reverting
if (balanceOf(from) < amount) {
return false;
}
return super.transferFrom(from, to, amount);
}
}
​
contract ContestManagerPoCTest is Test {
ContestManager public manager;
MockERC20isNonReverting public token;
address public owner = address(0x1);
address public player = address(0x2);
​
function setUp() public {
vm.startPrank(owner);
manager = new ContestManager();
token = new MockERC20isNonReverting();
vm.stopPrank();
}
​
function test_PoC_DoubleFundingAndSilentFailure() public {
address[] memory players = new address[](1);
players[0] = player;
uint256[] memory rewards = new uint256[](1);
rewards[0] = 1000;
​
vm.startPrank(owner);
​
// 1. Create a contest requiring 1000 tokens
address potAddress = manager.createContest(players, rewards, token, 1000);
// --- CASE 1: Silent Failure due to missing Approval ---
// Owner forgets to call token.approve() before funding.
// The transfer fails silently because the token returns false, but the transaction succeeds.
manager.fundContest(0);
// Verify that the Pot contract received 0 tokens even though the tx succeeded
assertEq(token.balanceOf(potAddress), 0);
console.log("Pot Balance after unapproved fundContest:", token.balanceOf(potAddress));
​
// --- CASE 2: Double Funding ---
// Owner now approves the tokens properly
token.approve(address(manager), 5000);
​
// First legitimate funding
manager.fundContest(0);
uint256 balanceAfterFirstFund = token.balanceOf(potAddress);
assertEq(balanceAfterFirstFund, 1000);
​
// Second accidental funding due to no state tracking
manager.fundContest(0);
uint256 balanceAfterSecondFund = token.balanceOf(potAddress);
// The Pot has now been funded twice, stealing extra tokens from the owner
assertEq(balanceAfterSecondFund, 2000);
console.log("Pot Balance after accidental double funding:", balanceAfterSecondFund);
​
vm.stopPrank();
}
}

Recommended Mitigation

Implement a state tracking mapping to prevent multiple funding phases and switch to OpenZeppelin's SafeERC20 wrapper library to handle token approvals and return values securely.

Solidity

+import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract ContestManager is Ownable {
using SafeERC20 for IERC20;
​
mapping(address => bool) public isContestFunded;
​
error ContestManager__AlreadyFunded();
​
// ...
​
function fundContest(uint256 index) public onlyOwner {
address contestAddress = contests[index];
if (isContestFunded[contestAddress]) {
revert ContestManager__AlreadyFunded();
}
​
Pot pot = Pot(contestAddress);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[contestAddress];
​
isContestFunded[contestAddress] = true;
​
// safeTransferFrom handles the boolean check and reverts on failure
token.safeTransferFrom(msg.sender, address(pot), totalRewards);
}
}
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!