Description
-
The Snow::buySnow function accepts both native ETH and WETH for payment. When msg.value exactly equals the fee, the user pays with ETH. Otherwise, the function falls through to the else branch and charges the user in WETH via safeTransferFrom.
-
When a user sends ETH with the call but the amount does not exactly match the fee, the ETH is accepted by the payable function and remains trapped in the contract, while the user is simultaneously charged the full fee in WETH. The user pays twice for a single Snow token.
function buySnow(uint256 amount) external payable canFarmSnow {
if (msg.value == (s_buyFee * amount)) {
_mint(msg.sender, amount);
} else {
i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
_mint(msg.sender, amount);
}
s_earnTimer = block.timestamp;
emit SnowBought(msg.sender, amount);
}
Risk
Likelihood:
-
A user who has pre-approved the Snow contract for WETH and calls buySnow with a non-exact ETH amount triggers the double payment
-
Sending any msg.value that does not precisely equal s_buyFee * amount activates the WETH branch while keeping the ETH
Impact:
-
User loses the ETH sent with the transaction — stuck permanently in the Snow contract
-
User is also charged the full fee in WETH — paying twice for one Snow token
-
The trapped ETH can only be recovered by the fee collector via collectFee(), not by the victim
Proof of Concept
pragma solidity ^0.8.24;
import {Test, console2} from "forge-std/Test.sol";
import {Snow} from "../src/Snow.sol";
import {MockWETH} from "../src/mock/MockWETH.sol";
contract PoC_H04 is Test {
Snow snow;
MockWETH weth;
address victim;
address collector;
function setUp() public {
weth = new MockWETH();
collector = makeAddr("collector");
snow = new Snow(address(weth), 5, collector);
victim = makeAddr("victim");
}
function test_H04_BuySnowEthLoss() public {
uint256 fee = snow.s_buyFee();
deal(victim, 1 ether);
weth.mint(victim, fee);
console2.log("Victim ETH balance before:", victim.balance);
console2.log("Victim WETH balance before:", weth.balanceOf(victim));
console2.log("Buy fee:", fee);
vm.startPrank(victim);
weth.approve(address(snow), fee);
snow.buySnow{value: 1 ether}(1);
vm.stopPrank();
console2.log("Victim ETH balance after:", victim.balance);
console2.log("Victim WETH balance after:", weth.balanceOf(victim));
console2.log("Snow contract ETH balance:", address(snow).balance);
assertEq(victim.balance, 0, "Victim lost all ETH");
assertEq(weth.balanceOf(victim), 0, "Victim also paid WETH");
assertEq(address(snow).balance, 1 ether, "ETH stuck in Snow contract");
assertEq(snow.balanceOf(victim), 1, "Only got 1 Snow token for double payment");
console2.log("RESULT: Victim paid 1 ETH + 5e18 WETH for 1 Snow token (should only cost one)");
}
}
Output:
[PASS] test_H04_BuySnowEthLoss() (gas: 209467)
Logs:
Victim ETH balance before: 1000000000000000000
Victim WETH balance before: 5000000000000000000
Buy fee: 5000000000000000000
Victim ETH balance after: 0
Victim WETH balance after: 0
Snow contract ETH balance: 1000000000000000000
RESULT: Victim paid 1 ETH + 5e18 WETH for 1 Snow token (should only cost one)
Recommended Mitigation
Add an explicit else if (msg.value == 0) guard for the WETH payment path, and a final else branch that reverts with S__InvalidPayment() when ETH is sent but does not match the exact fee. This prevents the function from silently accepting ETH while simultaneously charging WETH, ensuring users can only pay through one method per call.
function buySnow(uint256 amount) external payable canFarmSnow {
if (msg.value == (s_buyFee * amount)) {
_mint(msg.sender, amount);
- } else {
+ } else if (msg.value == 0) {
i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
_mint(msg.sender, amount);
+ } else {
+ revert S__InvalidPayment();
}
s_earnTimer = block.timestamp;
emit SnowBought(msg.sender, amount);
}
