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

Unchecked ERC20 transfer/transferFrom return values in _transferReward and fundContest allow silent failures and phantom-funded pots

bytethebuilder

Root + Impact

Description

The protocol moves tokens via raw ERC20 calls and ignores the boolean return value in three places:

// Pot.sol
function _transferReward(address player, uint256 reward) internal {
@> i_token.transfer(player, reward); // return value ignored
}
​
function closePot() external onlyOwner {
...
@> i_token.transfer(msg.sender, managerCut); // return value ignored
...
}
​
// ContestManager.sol
function fundContest(uint256 index) public onlyOwner {
...
@> token.transferFrom(msg.sender, address(pot), totalRewards); // return value ignored
}

The ERC20 standard allows a token to signal failure by returning false instead of reverting. Several real tokens behave this way. Because these calls do not check the return value (and do not use SafeERC20), a failed transfer is treated as success. The most damaging case is fundContest: it can complete "successfully" while moving zero tokens, leaving a Pot that is recorded as funded (contestToTotalRewards set, remainingRewards == totalRewards) but actually holds nothing - every later claimCut() then reverts.

Risk

Likelihood: Low - only manifests with ERC20 tokens that return false on failure rather than reverting.

Impact: Low - no direct theft, but it produces phantom-funded pots and silently-failed reward payouts, breaking the protocol's core accounting for such tokens.

Proof of Concept

A token whose transferFrom returns false (without reverting) lets fundContest "succeed" while transferring nothing. Runnable Foundry test (add the mock + test to test/TestMyCut.t.sol):

// minimal ERC20 that always returns false on transfers (no revert)
contract FalseToken {
mapping(address => uint256) public balanceOf;
function mint(address to, uint256 amt) external { balanceOf[to] += amt; }
function approve(address, uint256) external returns (bool) { return true; }
function transfer(address, uint256) external returns (bool) { return false; }
function transferFrom(address, address, uint256) external returns (bool) { return false; }
}
​
function test_PoC_uncheckedTransferReturn() public {
vm.startPrank(user);
address conMan2 = address(new ContestManager());
FalseToken ft = new FalseToken();
ft.mint(user, 1000);
ft.approve(conMan2, 1000);
​
address c = ContestManager(conMan2).createContest(players, rewards, IERC20(address(ft)), 4);
// fundContest does NOT revert even though transferFrom returns false and moves nothing
ContestManager(conMan2).fundContest(0);
vm.stopPrank();
​
// pot is "funded" in bookkeeping but holds zero tokens
assertEq(ft.balanceOf(c), 0);
assertEq(ContestManager(conMan2).getContestTotalRewards(c), 4);
}

Run forge test --mt test_PoC_uncheckedTransferReturn -vv; it passes, proving a phantom-funded pot.

Recommended Mitigation

Use OpenZeppelin's SafeERC20 wrappers (safeTransfer / safeTransferFrom), which revert when a token returns false or returns no data. This makes every transfer either succeed or revert, so accounting can never desync.

import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
import {IERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/IERC20.sol";
​
contract Pot {
using SafeERC20 for IERC20;
​
function _transferReward(address player, uint256 reward) internal {
i_token.safeTransfer(player, reward); // reverts on failure
}
}
​
// ContestManager.fundContest
token.safeTransferFrom(msg.sender, address(pot), totalRewards); // reverts on failure

Apply the same safeTransfer change to the managerCut transfer in closePot().

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!