Submission Details
Impact:
Likelihood:
Unsafe Token Transfer in `Snow::collectFee` function causing potential silent failures and loss of funds
Root + Impact
Description
The `collectFee()` function uses the unsafe `i_weth.transfer()` method instead of SafeERC20's `safeTransfer()` when transferring WETH tokens to the collector. This creates a critical vulnerability because many ERC20 tokens do not return boolean values on transfer operations, which can lead to silent failures where the transfer fails but the transaction doesn't revert.
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
//q what happens when the transfer fails?
//@audit tokens like USDT do not return anything if fail
@> i_weth.transfer(s_collector, collection); // Unsafe transfer
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
Risk
Likelihood:
-
Major tokens like USDT don't return booleans, so it will silently fail on transfer
Impact:
- Silent failures: WETH transfers can fail without reverting the transaction, leaving tokens stuck in the contract while appearing to succeed
- Loss of funds: Accumulated WETH fees become permanently inaccessible if transfers consistently fail
- Collector loss: The fee collector may believe they received all fees when only ETH was successfully transferred
Proof of Concept
Run the test in TestSnow.t.sol: showing that:
-
ETH collection succeeds (5 ETH goes to collector)
-
WETH transfer fails silently (1000 WETH stays in contract)
-
Overall transaction appears successful
-
Collector loses the WETH fees
function testSilentFailure() public {
// 1. Contract accumulates 1000 WETH in fees
uint256 initialWethBalance = 1000 ether;
weth.mint(address(snow), initialWethBalance);
// 2. Contract also has 5 ETH
vm.deal(address(snow), 5 ether);
vm.mockCall(
address(weth),
abi.encodeWithSelector(weth.transfer.selector, collector, initialWethBalance),
abi.encode(false) // Make transfer return false
);
// 3. Collector attempts to collect fees
vm.prank(collector);
snow.collectFee(); // Transaction succeeds
// 4. ETH is collected successfully
assertEq(collector.balance, 5 ether);
// 5. But WETH remains stuck in contract (silent failure)
assertEq(weth.balanceOf(address(snow)), initialWethBalance); // Still 1000 WETH
assertEq(weth.balanceOf(collector), 0); // Collector gets 0 WETH
console2.log("Initial Weth balance:", initialWethBalance);
// Transaction appeared successful but WETH was not transferred
console2.log("Transaction appeared successful but WETH was not transferred");
}
Recommended Mitigation
Use SafeERC20's safeTransfer(): Replace the unsafe transfer with the safe version that properly handles non-compliant tokens:
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
//q what happens when the transfer fails?
//@audit tokens like USDT do not return anything if fail on transfer
- 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!!!");
}
Rewards breakdown
nSLOC
215This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.