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

Unsafe ERC20 Transfer in Fee Collection May Cause Silent Failure

zanyjoy408

Description

  • The Snow.sol contract uses SafeERC20 for WETH interactions in buySnow(), but inconsistently uses the unsafe transfer() function in collectFee().

  • Some ERC20 tokens (including some WETH implementations) do not return a boolean on transfer(), or return false on failure instead of reverting. Using the raw transfer() function without checking the return value can cause silent failures.

// src/Snow.sol:19
using SafeERC20 for IERC20;
​
// src/Snow.sol:83-84 - Correctly uses safeTransferFrom
i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
​
// src/Snow.sol:101-107
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
// @> Uses unsafe transfer instead of safeTransfer
i_weth.transfer(s_collector, collection);
​
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}

Risk

Likelihood:

  • The constructor validates that _weth is not address(0), but does not verify it's a compliant ERC20

  • Non-standard WETH implementations or wrapper tokens may not follow the expected return value pattern

  • The likelihood increases if the contract is deployed on L2s or sidechains with different WETH implementations

Impact:

  • WETH fee collection may silently fail while the function reports success

  • Accumulated WETH fees could become stuck in the contract

  • The collector believes fees were received when they were not

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test} from "forge-std/Test.sol";
import {Snow} from "../src/Snow.sol";
​
// Mock token that returns false on transfer (non-reverting failure)
contract BadWETH {
mapping(address => uint256) public balanceOf;
​
function transfer(address, uint256) external pure returns (bool) {
return false; // Silently fails
}
​
function transferFrom(address from, address to, uint256 amount) external returns (bool) {
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
​
function mint(address to, uint256 amount) external {
balanceOf[to] += amount;
}
}
​
contract UnsafeTransferPoC is Test {
Snow snow;
BadWETH badWeth;
address collector = makeAddr("collector");
​
function setUp() public {
badWeth = new BadWETH();
snow = new Snow(address(badWeth), 1, collector);
}
​
function testSilentTransferFailure() public {
// Add WETH to the Snow contract
badWeth.mint(address(snow), 100 ether);
​
uint256 collectorBalanceBefore = badWeth.balanceOf(collector);
​
// Collect fees - won't revert but transfer returns false
vm.prank(collector);
snow.collectFee(); // This succeeds but WETH wasn't actually transferred
​
uint256 collectorBalanceAfter = badWeth.balanceOf(collector);
​
// Collector received nothing
assertEq(collectorBalanceAfter, collectorBalanceBefore);
// WETH still stuck in contract
assertEq(badWeth.balanceOf(address(snow)), 100 ether);
}
}

Recommended Mitigation

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