Submission Details
Impact:
Likelihood:
Unchecked return value on WETH transfer in `Snow::collectFee` can silently fail and leave fees stuck in the contract
Unchecked return value on WETH transfer in `Snow::collectFee` can silently fail and leave fees stuck in the contract
Description
The `collectFee` function checks to see if the ETH transfer fails from the boolean return value, but does not do the same for WETH.
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
i_weth.transfer(s_collector, collection);
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
Risk
Likelihood:
Instead of reverting on failure, a non-standard WETH implementation may return false with no check.
Impact:
This would result in a succesful ETH transfer and leave WETH temporarily stuck in the contract.
Proof of Concept
Add the following statement to the imports of `TestSnow.t.sol`
```solidity
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
```
Add the following contract to `TestSnow.t.sol`
```solidity
contract FalseReturnERC20 is ERC20 {
constructor() ERC20("Fake WETH", "fWETH") {}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
/// @dev Overridden to simulate a non-standard token that
/// returns false instead of reverting on failure.
function transfer(address to, uint256 amount) public pure override returns (bool) {
return false; // no revert — silently signals failure
}
}
```
Add the following function to `TestSnow` contract:
```solidity
function test_CollectFee_SilentlyFailsAndLeavesWethStuck() public {
FalseReturnERC20 fakeWeth = new FalseReturnERC20();
// new snow with fake weth
snow = new Snow(address(fakeWeth), FEE, collector);
// Fund the Snow contract with fees to collect
fakeWeth.mint(address(snow), 100 ether);
uint256 contractWethBefore = fakeWeth.balanceOf(address(snow));
uint256 collectorWethBefore = fakeWeth.balanceOf(collector);
vm.prank(collector);
snow.collectFee(); // should NOT revert, despite WETH transfer failing
uint256 contractWethAfter = fakeWeth.balanceOf(address(snow));
uint256 collectorWethAfter = fakeWeth.balanceOf(collector);
// Assert: WETH never moved, despite collectFee() completing without error
assertEq(contractWethAfter, contractWethBefore, "WETH balance should be unchanged");
assertEq(collectorWethAfter, collectorWethBefore, "Collector should not have received WETH");
console2.log("WETH stuck in contract:", contractWethAfter);
}
```
In the terminal, run the test:
```bash
forge test --mt test_CollectFee_SilentlyFailsAndLeavesWethStuck -vvvv
```
Test will pass if WETH is stuck in the contract and console log stuck amount.
Recommended Mitigation
Add a require statement to check if `transfer` returns false. Also, use `safeTransfer` instead of `transfer`.
```diff
function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
- i_weth.transfer(s_collector, collection);
+ if(collection > 0){
+ bool wethCollected = i_weth.safeTransfer(s_collector, collection);
+ require(wethCollected, "Fee collection failed!!!");
+ }
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
```
Rewards breakdown
This contest has a flat reward structure with the following payouts:
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.