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

Unchecked ERC20 transfer() Return Value in collectFee()

sp0ck0x

Root + Impact

Description

Snow correctly uses SafeERC20.safeTransferFrom() when pulling WETH from buyers in buySnow(), but switches to the
raw IERC20.transfer() call — without checking its return value — when sweeping accumulated fees in collectFee().
Tokens that signal failure by returning false rather than reverting (e.g. USDT, BNB, and other non-compliant
ERC20s) would silently fail: the transfer would appear to succeed, ETH would be sent to the collector, and the
WETH balance would remain locked in the contract with no recovery path.

While the contract currently hardcodes i_weth to MockWETH/canonical WETH (which reverts rather than returning
false), the inconsistency creates a latent risk: if i_weth is ever pointed at a different token, or this pattern
is replicated for another asset, fees silently disappear.

// Snow.sol:101-107
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
@> i_weth.transfer(s_collector, collection); // raw transfer, return value ignored
// SafeERC20 already imported and used elsewhere
​
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}

Risk

Risk

Likelihood:

  • When i_weth is set to any ERC20 token that returns false on failure rather than reverting

  • When the contract is reused or forked with a different token address substituted for WETH

  • When the collector calls collectFee() during any edge-case token state (e.g. paused USDT)

Impact:

  • Accumulated WETH fees silently remain in the contract with no user-facing revert

  • ETH is sent to the collector (via the call below) while WETH transfer failure goes unnoticed

  • Fees are permanently locked — collectFee() has no retry logic and i_weth has no admin withdrawal fallback

Proof of Concept

Static confirmation — the pattern is directly visible at Snow.sol:103. The SafeERC20 wrapper is imported and
active at line 19 (using SafeERC20 for IERC20) but not applied to this call site. Slither flags this as
unchecked-transfer.

To demonstrate the silent-failure path with a non-compliant token:

// 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";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
/// @dev Token that returns false on transfer instead of reverting
contract ReturnFalseToken is ERC20 {
constructor() ERC20("Bad", "BAD") {}
function mint(address to, uint256 amt) external { _mint(to, amt); }
function transfer(address, uint256) public pure override returns (bool) {
return false; // non-compliant: signal failure without revert
}
function transferFrom(address from, address to, uint256 amt) public override returns (bool) {
_transfer(from, to, amt);
return true;
}
}
​
contract PoC_SA06 is Test {
ReturnFalseToken badToken;
Snow snow;
address collector = makeAddr("collector");
​
function setUp() public {
badToken = new ReturnFalseToken();
// Deploy Snow with the non-compliant token as WETH
snow = new Snow(address(badToken), 1, collector);
// Simulate accumulated fees: send bad tokens directly to Snow
badToken.mint(address(snow), 100e18);
}
​
function test_info_SA06_UncheckedTransfer_StaticConfirmation() public {
uint256 collectorBefore = badToken.balanceOf(collector);
uint256 contractBefore = badToken.balanceOf(address(snow));
​
// collectFee() does NOT revert despite transfer returning false
vm.prank(collector);
vm.deal(collector, 0);
snow.collectFee();
​
uint256 collectorAfter = badToken.balanceOf(collector);
uint256 contractAfter = badToken.balanceOf(address(snow));
​
// Transfer silently failed -- fees remain locked in contract
assertEq(collectorAfter, collectorBefore, "collector received nothing");
assertEq(contractAfter, contractBefore, "fees still locked in contract");
​
console2.log("Fees locked in contract:", contractAfter);
console2.log("[CONFIRMED] SA-06: transfer() return value unchecked -- silent failure possible");
}
}
​

Recommended Mitigation

Replace the raw transfer() call with the safeTransfer() wrapper already imported via SafeERC20. This wraps the
call in a check that reverts if the return value is false or if the call reverts — matching the pattern already
used in buySnow().

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 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!