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

Unchecked ERC20 transfer and transferFrom return values allow silent transfer failures, causing permanent state corruption and fund loss

squashingbugs007

Root + Impact

Description

  • Normal behavior: Every token transfer in the protocol is critical. Funding a pot, paying player rewards, and paying the manager cut all move real value. A failed transfer should revert the entire transaction, ensuring the contract state never diverges from the actual token balances.

  • The issue: The ERC20 standard does not guarantee that transfer() and transferFrom() revert on failure. Some widely used tokens, including USDT, BNB, and others, return false on failure instead of reverting. The protocol uses raw .transfer() and .transferFrom() calls without checking the return value anywhere.

  • When a transfer silently fails and returns false, the transaction continues executing normally. This means: a player's reward transfer can fail silently while playersToRewards[player] has already been set to zero. The player loses their reward permanently with no way to retry. The manager cut can fail silently while remainingRewards is consumed. The funding transfer in fundContest() can fail silently while the pot contract believes it is funded and allows the contest to proceed against an empty balance.

  • The protocol states it supports "Standard ERC20 Tokens Only". However USDT is one of the most widely used ERC20 tokens and exhibits exactly this behavior.

// Pot.sol line 55 — manager cut transfer, return value ignored
i_token.transfer(msg.sender, managerCut);
​
// Pot.sol line 65 — player reward transfer, return value ignored
i_token.transfer(player, reward);
​
// ContestManager.sol line 37 — funding transfer, return value ignored
token.transferFrom(msg.sender, address(pot), totalRewards);

Risk

Likelihood:

  • Triggered whenever the protocol is used with a non-reverting ERC20 token such as USDT.

  • The protocol's own compatibility statement ("Standard ERC20 Tokens") does not exclude these tokens.

  • Any token transfer failure due to token-level restrictions (blacklists, pauses, caps) would trigger this silently.

Impact:

  • Player rewards zeroed in state but never actually transferred. Permanent fund loss with no retry mechanism.

Pot funded in state but tokens never received. Contest runs against zero balance, all subsequent transfers fail silently.

  • Manager cut recorded as paid but never received. Silent accounting corruption throughout.

Proof of Concept

The following demonstrates using a mock token that returns false instead of reverting. The player's reward is wiped from state but never transferred.

// Mock token that returns false instead of reverting
contract ReturnFalseToken is ERC20 {
constructor() ERC20("Fake", "FK") {}
function transfer(address, uint256) public pure override returns (bool) {
return false; // simulates USDT behavior on failure
}
}
​
function test_silentTransferFailureLosesFunds() public {
ReturnFalseToken badToken = new ReturnFalseToken();
address[] memory players = new address[](1);
uint256[] memory rewards = new uint256[](1);
players[0] = alice;
rewards[0] = 100e18;
​
Pot pot = new Pot(players, rewards, badToken, 100e18);
// Note: fundContest also silently fails but state says funded
​
vm.prank(alice);
pot.claimCut();
// transfer returns false — alice receives nothing
// but playersToRewards[alice] is now 0
// alice cannot call claimCut() again — reward is gone
​
assertEq(badToken.balanceOf(alice), 0); // alice got nothing
assertEq(pot.checkCut(alice), 0); // but her reward is zeroed
// funds are permanently lost
}

Recommended Mitigation

Replace all raw transfer() and transferFrom() calls with OpenZeppelin's SafeERC20 wrapper, which checks return values and reverts on failure. This is the industry standard for ERC20 transfers in production contracts. Using SafeERC20 ensures any transfer failure, whether from a returning-false token or any other failure mode. It reverts the transaction and preserves contract state integrity.

+ import {SafeERC20} from "lib/openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract Pot is Ownable(msg.sender) {
+ using SafeERC20 for IERC20;
...
​
- i_token.transfer(msg.sender, managerCut);
+ i_token.safeTransfer(msg.sender, managerCut);
​
- i_token.transfer(player, reward);
+ i_token.safeTransfer(player, reward);
}
​
contract ContestManager is Ownable {
+ using SafeERC20 for IERC20;
...
​
- token.transferFrom(msg.sender, address(pot), totalRewards);
+ token.safeTransferFrom(msg.sender, address(pot), totalRewards);
}
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!