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

collectFee() does not check WETH transfer return value — fee collection silently fails

squashingbugs007

Root + Impact

Description

  • Normal behavior: collectFee() is intended to transfer all accumulated WETH fees from the Snow contract to the collector address.

  • The issue: i_weth.transfer(s_collector, collection) uses the raw IERC20.transfer() function whose return value is not checked. Some ERC20 tokens (notably non-standard implementations) return false on failure instead of reverting. If the WETH transfer fails silently, the function continues to send native ETH to the collector, giving the false impression that fees were collected when WETH remains stuck in the contract.

```solidity
// src/Snow.sol#101-107
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
// @> return value not checked — silent failure possible
i_weth.transfer(s_collector, collection);
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
```

Risk

Likelihood:

  • If WETH contract is upgraded or replaced with a non-standard implementation, transfer() may return false.

  • The collector calls this function to withdraw fees and silent failure means lost revenue.

Impact:

  • WETH fees are silently lost and collector receives ETH but not WETH

  • No error is raised and collector has no indication the WETH transfer failed

Proof of Concept

The following Foundry test demonstrates that collectFee() silently succeeds
even when the WETH transfer fails. A mock WETH token that returns false on
transfer is used to simulate the failure. The function completes without
reverting, the collector receives ETH but not WETH, and the contract
balance check shows WETH is still stuck in the contract with no error raised.

function testUncheckedTransferSilentFailure() public {
// Deploy Snow with MockWETH that returns false on transfer
MockWETH mockWeth = new MockWETH(); // transfer() returns false
Snow snow = new Snow(address(mockWeth), buyFee, collector);
// User buys Snow with WETH — fees accumulate in contract
vm.startPrank(user);
mockWeth.approve(address(snow), buyFee * 10);
snow.buySnow(10);
vm.stopPrank();
uint256 wethBalanceBefore = mockWeth.balanceOf(address(snow));
assertGt(wethBalanceBefore, 0); // WETH accumulated
// Collector calls collectFee — WETH transfer returns false silently
vm.prank(collector);
snow.collectFee(); // Does NOT revert
// WETH still stuck in contract — silent failure
uint256 wethBalanceAfter = mockWeth.balanceOf(address(snow));
assertEq(wethBalanceAfter, wethBalanceBefore); // WETH never moved
}

Recommended Mitigation

The fix replaces the raw IERC20.transfer() call with SafeERC20.safeTransfer()
which is already imported and used correctly elsewhere in Snow.sol. SafeERC20
wraps the transfer call and reverts if the return value is false or if the
token does not return a value at all. This brings collectFee() into
consistency with buySnow() which already uses i_weth.safeTransferFrom()
correctly. No new imports are required, SafeERC20 is already in scope.

function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
- // @> raw transfer — return value silently ignored
- i_weth.transfer(s_collector, collection);
+ // @> safeTransfer reverts on failure — already used in buySnow()
+ // SafeERC20 is already imported — no new dependencies needed
+ 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!