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

Unhandled ETH Transfer Failure via call in collectFee() — Possible Fund Loss

0xsiddhu

Description

Root Cause + Impact Description

The Snow contract's collectFee() function uses a low-level .call{value: ...}("") to forward all ETH held by the contract to the s_collector address. While it uses require(collected, ...) to check the return status of the call, this does not guarantee that the transfer was safe or that the funds are not stuck.

In a scenario where the s_collector is a contract with either no receive()/fallback() function or with logic that reverts or consumes too much gas (e.g., reentrancy), the ETH transfer will fail. Although the function correctly reverts on failure, repeated calls will continue to fail, potentially locking all ETH in the contract permanently if a valid collector address is never restored.

Normal Behavior

The collectFee() function is expected to send all held WETH and ETH from the contract to the s_collector address safely and atomically.

Vulnerable Behavior

If the s_collector is non-payable, reverts during receive, or is a contract with fallback/receive functions consuming excessive gas or performing reentrancy, the call will fail. The function will revert, but the ETH remains stuck and inaccessible.

Vulnerable Code Snippet

function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection); //@audit-info unchecked return
​
// @> Low-level call to transfer ETH with no gas constraints or fallback safety
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}

Risk

Likelihood

  • This will occur when a collector contract lacks a receive() or fallback() function or includes logic that makes it non-payable or gas-intensive.

  • If a collector contract is upgraded or replaced with a hostile or broken address, fee collection can permanently fail.

Impact

  • Loss of access to ETH funds in the Snow contract.

  • Repeated failures to collect fees can render the collectFee() function useless and lead to operational degradation of the system.

Proof of Concept

// inside test folder
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.24;
​
import "forge-std/Test.sol";
import "../src/Snow.sol";
import "../src/mock/BadWETH.sol";
import "forge-std/console.sol";
import "../src/mock/NonPayableCollector.sol";
​
​
contract SnowFeeTest is Test {
Snow snow;
BadWETH badWeth;
NonPayableCollector npc;
​
function setUp() public {
badWeth = new BadWETH();
​
snow = new Snow(address(badWeth), 1, address(this));
​
npc = new NonPayableCollector(snow);
​
vm.prank(snow.owner());
snow.changeCollector(address(npc));
​
badWeth.mint(address(snow), 1000 ether);
vm.deal(address(snow), 1 ether);
}
​
​
​
function test_collectFeeFails() public {
vm.expectRevert(bytes("Fee collection failed!!!"));
npc.triggerCollect(); // call throug rejecting contract
​
uint256 wethBalance = badWeth.balanceOf(address(snow));
uint256 ethBalance = address(snow).balance;
uint256 collectorBalance = address(npc).balance;
​
console.log(" WETH balance in Snow: ", wethBalance);
console.log(" ETH balance in Snow: ", ethBalance);
console.log(" ETH balance in Collector: ", collectorBalance);
​
assertEq(wethBalance, 1000 ether, " WETH not drained");
assertEq(ethBalance, 1 ether, " ETH not drained");
assertEq(collectorBalance, 0, " Collector should not receive ETH");
}
​
​
}
​
// mock contract at => src/mock/BadWETH.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.24;
​
import "forge-std/Test.sol";
import "../Snow.sol";
​
// Non-compliant token that fails silently
contract BadWETH is ERC20("BadWETH", "bWETH") {
constructor() {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
​
function transfer(address to, uint256 amount) public override returns (bool) {
return false; // simulate silent failure
}
}
​
// Non-payable contract to simulate ETH call failur

Output

Ran 1 test for test/TestUncheckReturn.sol:SnowFeeTest
[PASS] test_collectFeeFails() (gas: 68937)
Logs:
WETH balance in Snow: 1000000000000000000000
ETH balance in Snow: 1000000000000000000
ETH balance in Collector: 0
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 563.21µs (109.83µs CPU time)
​
Ran 1 test suite in 3.27ms (563.21µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
​

Recommended Mitigation

  • Avoid using .call{value: ...}("") directly for ETH transfers without explicit gas forwarding or security checks.

  • Instead:

    • Use a pull pattern where the collector withdraws ETH when desired.

    • Or leverage OpenZeppelin’s Address.sendValue() which reverts if the transfer fails and mitigates gas stipend issues.

    • Consider implementing a SafeTransferLib-style ETH transfer using call, with checks for contract reentrancy or non-payable recipients.

Example using OZ's safer pattern:

import {Address} from "@openzeppelin/contracts/utils/Address.sol";
​
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection);
Address.sendValue(payable(s_collector), address(this).balance);
}
Updates

Lead Judging Commences

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

Support

FAQs

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