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

Unsafe Token Transfer in Fee Collection + Silent Failure

cheesesteak

Root + Impact

Description

  • The collectFee() function uses an unsafe transfer() call for WETH tokens that doesn't check the return value, allowing the function to succeed even when the token transfer fails silently.

  • The collectFee() function should safely transfer all accumulated WETH fees to the collector address and revert if the transfer fails to ensure fee collection integrity.

// Root cause in the codebase with @> marks to highlight the relevant section
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
// @> Unsafe transfer - doesn't check return value
i_weth.transfer(s_collector, collection);
​
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}

Risk

Likelihood:

  • Occurs when WETH token contract returns false instead of reverting on failed transfers

  • Some ERC20 implementations (including potential WETH variants) return false on failure rather than reverting

Impact:

  • Fees remain stuck in the contract while the collector believes they were successfully collected

  • No event emission or error indication when WETH transfer fails

  • Loss of transparency in fee collection process

  • Potential permanent loss of fees if collector role cannot be changed

Proof of Concept

The POC demonstrates that with this WETH implementation it returns false instead of reverting. Then the collectFee() function will succeed while leaving WETH tokens stuck in the contract, creating a false sense of successful fee collection.

// Malicious WETH that fails transfers silently (returns false instead of reverting)
contract MaliciousWETH is ERC20Mock {
bool public shouldFailTransfer = false;
function setShouldFailTransfer(bool _shouldFail) external {
shouldFailTransfer = _shouldFail;
}
function transfer(address to, uint256 amount) public override returns (bool) {
if (shouldFailTransfer) {
return false; // Fail silently without reverting
}
return super.transfer(to, amount);
}
}
​
function testUnsafeWETHTransferVulnerability() public {
// Setup Snow contract with malicious WETH
MaliciousWETH maliciousWeth = new MaliciousWETH();
Snow snow = new Snow(address(maliciousWeth), 1, collector); // 1 wei * PRECISION = 1 ether fee
// User buys Snow with ETH, putting ETH in contract
vm.deal(user, 10 ether);
vm.prank(user);
snow.buySnow{value: 5 ether}(5); // 5 tokens * 1 ether = 5 ether
// Simulate WETH fees in contract (from previous WETH purchases)
maliciousWeth.mint(address(snow), 100 ether);
console.log("Before attack:");
console.log("- Contract WETH balance:", maliciousWeth.balanceOf(address(snow))); // 100 ether
console.log("- Collector WETH balance:", maliciousWeth.balanceOf(collector)); // 0
// ATTACK: Make WETH transfers return false
maliciousWeth.setShouldFailTransfer(true);
// Collector attempts to collect fees
vm.prank(collector);
snow.collectFee(); // This succeeds even though WETH transfer failed!
console.log("After collectFee():");
console.log("- Contract WETH balance:", maliciousWeth.balanceOf(address(snow))); // Still 100 ether
console.log("- Collector WETH balance:", maliciousWeth.balanceOf(collector)); // Still 0
console.log("- Collector ETH balance:", collector.balance); // 5 ether (ETH was collected)
// Verify vulnerability
assertEq(maliciousWeth.balanceOf(address(snow)), 100 ether); // WETH stuck in contract
assertEq(maliciousWeth.balanceOf(collector), 0); // Collector got no WETH
assertEq(collector.balance, 5 ether); // But got ETH (partial success)
// VULNERABILITY CONFIRMED:
// - WETH transfer failed silently (returned false)
// - collectFee() succeeded anyway
// - 100 ether WETH remains stuck in contract
// - Collector believes fees were collected successfully
}

Recommended Mitigation

use safeTransfer, that will revert if failure occurs.

- remove this code
+ add this code
+ import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract Snow is ERC20, Ownable {
+ 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!!!");
+
+ emit FeeCollected();
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge
5 months ago
yeahchibyke Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.