MyCut

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

Unchecked ERC20 transfer return values can silently fail with non-standard tokens

Root + Impact

Description

The protocol moves reward tokens with raw transfer/transferFrom calls and never checks the boolean return value. Some ERC20 tokens (e.g. USDT and other non-standard implementations) return false on failure instead of reverting. With such tokens, a failed transfer is treated as successful, corrupting the accounting: rewards are marked as paid/claimed while no tokens actually moved.

// Pot.sol
function _transferReward(address player, uint256 reward) internal {
i_token.transfer(player, reward); // @> return value ignored
}
// ContestManager.sol
token.transferFrom(msg.sender, address(pot), totalRewards); // @> return value ignored

Risk

Likelihood:

  • Occurs when the pot's reward token is a non-standard ERC20 that returns false instead of reverting on failure.

Impact:

  • A failed transfer is treated as success: playersToRewards is zeroed and the claimant is recorded, but no tokens are received — the player loses their reward and accounting diverges from real balances.

Proof of Concept

Note: with a non-standard token that returns false instead of reverting, claimCut() zeroes playersToRewards and records the claimant while no tokens move, since the transfer return value is never checked. Requires a mock false-returning ERC20 to run; the missing return-value check is evident from the source.

function test_PoC_UncheckedTransferSilentFail() public {
// Using a non-standard ERC20 that returns false on failure instead of reverting.
// (e.g. a mock mimicking USDT-style behavior)
address player = i_players[0];
uint256 rewardBefore = pot.checkCut(player);
assertGt(rewardBefore, 0);
// Simulate a transfer that returns false (e.g. pot underfunded / token quirk)
// claimCut zeroes the reward and records the claimant BEFORE the transfer result matters
vm.prank(player);
pot.claimCut();
// BUG: reward marked as claimed (0) even if the token transfer returned false
assertEq(pot.checkCut(player), 0); // state says "paid"
// but player's balance did NOT increase (transfer silently failed)
assertEq(token.balanceOf(player), 0); // no tokens received
}

Recommended Mitigation

Use OpenZeppelin's SafeERC20 (safeTransfer / safeTransferFrom), which reverts on failure and handles non-standard tokens correctly.

+ using SafeERC20 for IERC20;
- i_token.transfer(player, reward);
+ i_token.safeTransfer(player, reward);
Updates

Lead Judging Commences

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