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

[M-4] Unchecked ERC20 Transfer Return Values

jck

[M-4] Unchecked ERC20 Transfer Return Values

Description

  • Normal behavior: ERC20 transfers should either succeed or revert; the caller should know the transfer outcome.

  • Issue: The Pot contract calls i_token.transfer(player, reward) without checking the return value. Some ERC20 tokens (older or non-standard) return false instead of reverting on failure. Ignoring the return value allows the contract to behave as if the transfer succeeded, even though no tokens were actually transferred.

@> i_token.transfer(player, reward); // return value ignored

Risk

Likelihood:

  • High when interacting with non-standard ERC20 tokens that return false instead of reverting.

  • Common in third-party token integrations, DeFi contracts, or legacy ERC20 implementations.

Impact:

  • Users may be marked as paid without receiving tokens, resulting in funds remaining locked in the contract.

  • In extreme cases, this can break reward distribution logic and cause loss of user trust.

Severity: Medium (M)

Proof of Concept (PoC) Explanation

This PoC demonstrates the bug using a token that returns false on insufficient balance instead of reverting:

  1. Deploy a malicious ERC20 (FalseReturnERC20) that simulates the problematic behavior.

  2. Deploy the Pot contract with a reward schedule requiring 4 ETH total.

  3. Fund the Pot with only 1 ETH, deliberately insufficient to pay Player1’s 3 ETH reward.

  4. Player1 claims their reward. The Pot contract ignores the false return, decrements internal remainingRewards, but Player1 receives no tokens.

  5. The test asserts that Player1’s token balance is less than the expected reward, proving the bug.

contract FalseReturnERC20 is IERC20 {
string public constant name = "FalseToken";
string public constant symbol = "FALSE";
uint8 public constant decimals = 18;
mapping(address => uint256) public balances;
​
constructor(address initialHolder, uint256 amount) {
balances[initialHolder] = amount;
}
​
function totalSupply() external pure returns (uint256) {
return 1e18;
}
​
function balanceOf(address account) external view returns (uint256) {
return balances[account];
}
​
function transfer(address to, uint256 amount) external returns (bool) {
if (balances[msg.sender] < amount) {
return false; // simulate non-standard behavior
}
balances[msg.sender] -= amount;
balances[to] += amount;
return true;
}
​
function allowance(address, address) external pure returns (uint256) {
return 0;
}
​
function approve(address, uint256) external pure returns (bool) {
return true;
}
​
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
if (balances[from] < amount) {
return false;
}
balances[from] -= amount;
balances[to] += amount;
return true;
}
}
​
// PoC Test
function test_UncheckedERC20ReturnValue_PoC() public {
// Deploy a token that returns false on transfer
FalseReturnERC20 token = new FalseReturnERC20(address(this), 10 ether);
​
// Deploy the Pot with players and rewards
pot = new Pot(players, rewards, IERC20(token), totalRewards);
​
// Fund the pot with insufficient tokens (1 ether instead of 4)
token.transfer(address(pot), 1 ether);
​
// Player1 tries to claim 3 ether reward
vm.prank(player1);
pot.claimCut(); // transfer returns false but Pot ignores it
​
// Player1 balance should be less than expected reward
uint256 balance = token.balanceOf(player1);
console.log("Player1 balance:", balance);
console.log("Expected reward:", rewards[0]);
assertLt(balance, rewards[0], "PoC: player did not receive full reward due to unchecked transfer return");
​
// Internal accounting may incorrectly decrement remainingRewards
uint256 remaining = pot.getRemainingRewards();
console.log("Remaining rewards in pot:", remaining);
}

Expected outcome:

  • Player1 balance: 0 (or less than reward)

  • Pot.remainingRewards still decremented internally

  • Confirms that ignoring ERC20 return values can break reward distribution

Recommended Mitigation

Always check the return value of ERC20 transfers to prevent silent failures:

- i_token.transfer(player, reward);
+ require(i_token.transfer(player, reward), "Transfer failed");

Explanation:

  • require() ensures that the transaction reverts if the transfer fails, preventing inconsistencies between internal accounting and actual token balances.

  • This protects users from losing rewards due to non-standard ERC20 implementations.

  • Ensures deterministic and safe behavior, compliant with modern Solidity practices.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 1 day 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!