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

M-02: fundContest() Checks Balance But Uses transferFrom - Misleading Error

boujeeprosperous

Root + Impact

Root Cause

In ContestManager.sol lines 42-44, fundContest() checks the owner's token balance but then uses transferFrom() which requires allowance:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
contract ContestManager is Ownable {
// ... state variables ...
​
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
​
// @> ROOT CAUSE: Checks BALANCE but uses transferFrom (requires ALLOWANCE)
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
​
// @> Requires allowance but check was for balance
token.transferFrom(msg.sender, address(pot), totalRewards);
}
}

Impact

  • Misleading error message: Owner sees "InsufficientFunds" but actually has sufficient balance

  • Confusion: Owner checks balance, sees enough tokens, but transaction reverts

  • Debugging time wasted: Owner doesn't realize they need to approve() first

  • Functionality works if allowance is set correctly

Description

The fundContest() function checks the owner's token balance but then uses transferFrom() which requires allowance. If the owner has sufficient balance but hasn't approved the ContestManager, the balance check passes but transferFrom reverts with a misleading error.

// Lines 42-44 in ContestManager.sol
if (token.balanceOf(msg.sender) < totalRewards) {
revert ContestManager__InsufficientFunds();
}
token.transferFrom(msg.sender, address(pot), totalRewards); // Requires allowance!

Risk

Likelihood:
Medium - UX issue causing confusion. Doesn't lose funds but creates poor developer/user experience.
Impact:

  • Misleading error message: Owner sees "InsufficientFunds" but actually has sufficient balance

  • Confusion: Owner checks balance, sees enough tokens, but transaction reverts

  • Debugging time wasted: Owner doesn't realize they need to approve() first

  • Functionality works if allowance is set correctly

Proof of Concept

Before this goes out, I'd reframe the root cause from "misleading error message" to: the balance check is a redundant no-op against compliant tokens, and the unchecked transferFrom return value causes a silent, non-reverting funding failure against non-compliant tokens. That's a stronger, verifiable claim than the current narrative, and it changes the Impact table — this isn't purely a UX annoyance anymore, since a contest can end up in a "funded" state with an empty pot and no error to flag it. Severity is still your call, but I'd expect this to at least stay Medium and arguably push toward the fund-integrity side rather than pure UX.
The following Foundry test reproduces the vulnerability end-to-end. Save it as test/M02_FundContest_PoC.t.sol.

// SPDX-License-Identifier: MIT
usdc = new MockUSDC(); // owner receives 1,000,000 USDC
manager = new ContestManager();
// Create a contest whose Pot requires 100 USDC in total rewards
manager.createContest(usdc, 100 * 10 ** 18);
vm.stopPrank();
}
​
/// @notice Demonstrates the misleading error message
function testPoC_MisleadingErrorWhenAllowanceMissing() public {
vm.startPrank(owner);
​
// 1. Owner has plenty of balance (1,000,000 USDC)
uint256 ownerBal = usdc.balanceOf(owner);
console2.log("Owner USDC balance :", ownerBal);
assertGe(ownerBal, 100 * 10 ** 18);
​
// 2. Owner forgot to approve() the ContestManager
uint256 allowance = usdc.allowance(owner, address(manager));
console2.log("Owner allowance to mgr :", allowance);
assertEq(allowance, 0);
​
// 3. Calling fundContest — the inner check passes
// (balanceOf >= totalRewards), so the custom
// error ContestManager__InsufficientFunds is NOT triggered.
// Instead the call reverts deeper, with:
// "ERC20InsufficientAllowance(...)" — a totally different,
// non-obvious error from the perspective of an owner who
// was told "InsufficientFunds" by the custom error.
vm.expectRevert(); // reverts with ERC20InsufficientAllowance, NOT ContestManager__InsufficientFunds
manager.fundContest(0);
​
vm.stopPrank();
}
​
/// @notice Demonstrates that the function works fine when allowance is granted
function testPoC_SucceedsWhenApproveIsCalledFirst() public {
vm.startPrank(owner);
​
// Step 1 — approve
usdc.approve(address(manager), 100 * 10 ** 18);
​
// Step 2 — fundContest now succeeds
manager.fundContest(0);
​
// Step 3 — Pot holds the funds
address pot = manager.getContest(0);
uint256 potBal = usdc.balanceOf(pot);
console2.log("Pot balance after funding :", potBal);
assertEq(potBal, 100 * 10 ** 18);
​
vm.stopPrank();
}
}

▶️ How to Run

forge install OpenZeppelin/openzeppelin-contracts --no-commit
forge test --match-contract M02_FundContest_PoC -vvv
Expected console output:
Owner USDC balance : 1000000000000000000000000
Owner allowance to mgr : 0
Pot balance after funding: 100000000000000000000
The first test reverts with ERC20InsufficientAllowance (thrown from inside transferFrom), not ContestManager__InsufficientFunds — exactly the misleading behavior described in the report.

Recommended Mitigation

The best-practice fix is — wrap the transfer with OpenZeppelin's SafeERC20.safeTransferFrom and remove the manual balance check entirely, letting the ERC-20 implementation surface the precise failure reason:

​
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract ContestManager is Ownable {
using SafeERC20 for IERC20;
​
function fundContest(uint256 index) public onlyOwner {
Pot pot = Pot(contests[index]);
IERC20 token = pot.getToken();
uint256 totalRewards = contestToTotalRewards[address(pot)];
​
token.safeTransferFrom(msg.sender, address(pot), totalRewards);
}
}

  • safeTransferFrom reverts with a descriptive error (ERC20InsufficientAllowance or ERC20InsufficientBalance) — exactly matching what the contract is actually checking.

  • It also safely handles non-standard ERC-20s that return false from transferFrom instead of reverting.

  • Removing the redundant balanceOf check eliminates the misleading custom error path entirely.

Updates

Lead Judging Commences

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