Snowman Merkle Airdrop
AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

Unchecked ERC20 Transfer Return Value Can Cause Silent Fee Collection Failure

odinshell

Description

The Snow contract correctly imports and declares SafeERC20 for safe ERC20 interactions. The buySnow() function properly uses safeTransferFrom() to receive WETH from users. However, the collectFee() function inconsistently uses the raw transfer() method instead of safeTransfer().

Some ERC20 tokens (notably older versions of USDT and BNB) return false on failure instead of reverting. When using raw transfer(), the return value is not checked, allowing the transaction to succeed even when the token transfer fails. This results in the collector believing fees were collected when they remain locked in the contract.

// src/Snow.sol:15,19 - SafeERC20 is imported and declared
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
using SafeERC20 for IERC20;
​
// src/Snow.sol:83 - SafeERC20 IS used correctly here
i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
​
// src/Snow.sol:101-107 - SafeERC20 NOT used here
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection); // @> Raw transfer - return value NOT checked!
​
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}

Risk

Likelihood: Low

  • The vulnerability requires i_weth to be a non-standard ERC20 that returns false instead of reverting

  • Standard WETH implementations revert on failure, so the default deployment is likely safe

  • The bug manifests when the contract is deployed with non-compliant tokens

Impact: Medium

  • WETH fees can become permanently locked in the contract

  • Collector receives ETH fees but not WETH fees, with no indication of failure

  • The transaction completes "successfully" despite the transfer failing

  • No way to recover locked WETH without contract upgrade

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test, console2} from "forge-std/Test.sol";
import {Snow} from "../../src/Snow.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
// Malicious ERC20 that returns false instead of reverting
contract MaliciousWETH is IERC20 {
mapping(address => uint256) private _balances;
mapping(address => mapping(address => uint256)) private _allowances;
bool public transferShouldFail = false;
​
function setTransferShouldFail(bool _fail) external {
transferShouldFail = _fail;
}
​
function transfer(address, uint256) external view returns (bool) {
if (transferShouldFail) {
return false; // Silent failure - no revert!
}
return true;
}
​
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
_balances[from] -= amount;
_balances[to] += amount;
return true;
}
​
// ... other IERC20 functions ...
function balanceOf(address account) external view returns (uint256) { return _balances[account]; }
function allowance(address owner, address spender) external view returns (uint256) { return _allowances[owner][spender]; }
function approve(address spender, uint256 amount) external returns (bool) { _allowances[msg.sender][spender] = amount; return true; }
function totalSupply() external pure returns (uint256) { return 0; }
function mint(address to, uint256 amount) external { _balances[to] += amount; }
}
​
contract ExploitUncheckedTransfer is Test {
Snow snow;
MaliciousWETH weth;
address collector = makeAddr("collector");
address buyer = makeAddr("buyer");
​
function setUp() public {
weth = new MaliciousWETH();
snow = new Snow(address(weth), 5, collector);
uint256 fee = snow.s_buyFee();
weth.mint(buyer, fee);
}
​
function testExploit_WETHSilentlyLost() public {
uint256 fee = snow.s_buyFee();
​
// Buyer purchases Snow with WETH
vm.startPrank(buyer);
weth.approve(address(snow), fee);
snow.buySnow(1);
vm.stopPrank();
​
// Also add some ETH
address buyer2 = makeAddr("buyer2");
deal(buyer2, fee);
vm.prank(buyer2);
snow.buySnow{value: fee}(1);
​
console2.log("Contract WETH: %d", weth.balanceOf(address(snow)));
console2.log("Contract ETH: %d", address(snow).balance);
​
// Enable failure mode
weth.setTransferShouldFail(true);
​
// Collector calls collectFee
vm.prank(collector);
snow.collectFee(); // Does NOT revert!
​
// ETH was transferred, but WETH was NOT
assertEq(weth.balanceOf(address(snow)), fee); // WETH still in contract!
assertEq(weth.balanceOf(collector), 0); // Collector got nothing
assertEq(collector.balance, fee); // Only ETH was transferred
​
console2.log("[!] WETH transfer failed silently!");
console2.log("[!] WETH locked in contract: %d", weth.balanceOf(address(snow)));
}
}

Test Output:

[PASS] testExploit_WETHSilentlyLost() (gas: 298045)
Logs:
=== Exploit: WETH Silently Lost ===
​
[*] Contract has both WETH and ETH:
WETH: 5000000000000000000
ETH: 5000000000000000000
​
[*] collectFee() completed without revert!
[*] Collector WETH received: 0
[*] Collector ETH received: 5000000000000000000
[*] WETH still in contract: 5000000000000000000
​
[!] EXPLOIT SUCCESSFUL:
- ETH collected: YES
- WETH collected: NO (silently failed!)
- WETH permanently locked in contract

Foundry Linter Warning:

warning[erc20-unchecked-transfer]: ERC20 'transfer' and 'transferFrom' calls should check the return value
--> src/Snow.sol:103:9
|
103 | i_weth.transfer(s_collector, collection);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Recommended Mitigation

Use SafeERC20.safeTransfer() consistently throughout the contract:

// src/Snow.sol:101-107
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
- i_weth.transfer(s_collector, collection);
+ i_weth.safeTransfer(s_collector, collection);
​
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 9 days 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!