Snowman Merkle Airdrop

AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

Unchecked ERC20 Transfer Return Value in `collectFee()` Can Silently Fail and Lock WETH

Root + Impact

Description

According to the ERC20 standard, the `transfer()` function returns a boolean value indicating success or failure. Some ERC20 tokens return `false` on failure instead of reverting. The Snow contract's `collectFee()` function ignores the return value of `i_weth.transfer()`, allowing the transfer to silently fail while the function continues execution as if the transfer succeeded.Describe the normal behavior in one or more sentences.

Unchecked ERC20 Transfer Return Value in collectFee() Can Silently Fail and Lock WETH

akovachev7

Root + Impact

Description

According to the ERC20 standard, the transfer() function returns a boolean value indicating success or failure. Some ERC20 tokens return false on failure instead of reverting. The Snow contract's collectFee() function ignores the return value of i_weth.transfer(), allowing the transfer to silently fail while the function continues execution as if the transfer succeeded.

The contract inconsistently handles ERC20 transfers:

  • Uses SafeERC20.safeTransferFrom() in buySnow()

  • Uses raw transfer() without checking return value in collectFee()

In Snow.sol:

function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
//@audit: Unchecked return value - if transfer returns false, execution continues
@> i_weth.transfer(s_collector, collection);
(bool collected, ) = payable(s_collector).call{
value: address(this).balance
}("");
require(collected, "Fee collection failed!!!");
}

The contract already imports and declares using SafeERC20 for IERC20 but fails to use it consistently in collectFee().

Risk

Likelihood:

Medium: This depends on the specific WETH implementation used. Standard WETH implementations typically revert on failure, but

  • Some WETH variants or wrapped token implementations may return false instead

  • If the contract is deployed on different chains with different WETH implementations, behavior may vary

  • Future upgrades or changes to the WETH contract could introduce this behavior

  • Edge cases like blacklisted addresses or paused contracts could trigger silent failures

Impact:

High: When a transfer silently fails

  • WETH tokens remain permanently locked in the Snow contract

  • The collector believes fees were collected successfully (no revert)

  • ETH portion is transferred correctly, creating accounting inconsistencies

  • No mechanism exists to retry or recover the stuck WETH

  • Accumulated fees over time could represent significant value loss

  • The collector has no way to detect the failure without manually checking balances

  • Impact 2

Proof of Concept

The test demonstrates that when WETH transfer returns false, the function completes successfully while leaving WETH tokens stuck:

function testCollectFeeTransferSilentFail() public {
// Setup: Jerry buys with WETH, Victory buys with ETH
vm.startPrank(jerry);
weth.approve(address(snow), FEE);
snow.buySnow(1);
vm.stopPrank();
vm.prank(victory);
snow.buySnow{value: FEE}(1);
// Mock WETH transfer to return false (simulating a failed transfer)
vm.prank(collector);
vm.mockCall(
address(weth),
abi.encodeWithSelector(ERC20.transfer.selector, collector, FEE),
abi.encode(false) // Transfer returns false but doesn't revert
);
snow.collectFee(); // Function completes successfully!
// Results:
assert(weth.balanceOf(collector) == 0); // ❌ Collector didn't receive WETH
assert(collector.balance == FEE); // ✓ Collector received ETH
assert(address(snow).balance == 0); // ✓ ETH was transferred out
assert(weth.balanceOf(address(snow)) == FEE); // ❌ WETH stuck in contract
}

Expected behavior: If WETH transfer fails, the function should revert and prevent partial fee collection.

Recommended Mitigation

Use the SafeERC20 library that is already imported and configured in the contract. Replace the raw transfer() call with safeTransfer():

+ using SafeERC20 for IERC20;
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 4 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!