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

Unrefunded ETH in buySnow() Function Leading to Permanent Loss of User Funds

r7al38

Root + Impact

Root Cause: The buySnow function does not refund excess ETH when msg.value does not match the required fee, and instead keeps it in the contract.

Description

  • Normal behavior: When a user wants to buy Snow tokens, they should send exactly s_buyFee * amount in ETH. If they send the correct amount, they receive the tokens. If they don't send ETH, the contract tries to pull the equivalent value in WETH from their wallet.

  • the problem: If a user sends ETH but the amount is not exactly the required fee (for example, they send 2 ETH when only 1 ETH is needed), the contract still enters the WETH branch and pulls the required WETH from them. However, the ETH they sent is not returned – it stays stuck in the contract. Later, the collector can call collectFee and withdraw all ETH accumulated in the contract, effectively stealing the user’s funds.

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);
}
// @> No refund of msg.value when it's not zero but not equal to required amount
s_earnTimer = block.timestamp;
emit SnowBought(msg.sender, amount);
}

Risk

Likelihood: Medium

  • This will happen every time a user mistakenly sends an incorrect ETH amount (higher or lower) while calling buySnow.

  • It can also be triggered if the frontend miscalculates the required fee or if the user manually sends a wrong value.

Impact: High

  • Users lose the exact ETH they sent (which can be any amount).

  • Additionally, they also lose WETH equivalent to the required fee because the contract pulls WETH from them.

  • The collector can drain all trapped ETH at any time, making the loss irreversible.

Proof of Concept

The following Foundry test demonstrates the vulnerability:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import "forge-std/Test.sol";
import "../src/snow.sol";
​
contract WETH9 is ERC20 {
constructor() ERC20("Wrapped Ether", "WETH") {}
function deposit() public payable { _mint(msg.sender, msg.value); }
function withdraw(uint256 amount) external {
require(balanceOf(msg.sender) >= amount);
_burn(msg.sender, amount);
payable(msg.sender).transfer(amount);
}
receive() external payable { deposit(); }
}
​
contract SnowTest is Test {
Snow snow;
WETH9 weth;
address collector = makeAddr("collector");
address alice = makeAddr("alice");
​
uint256 constant PRECISION = 10 ** 18;
uint256 buyFee = 1;
uint256 amount = 1;
​
function setUp() public {
weth = new WETH9();
vm.prank(collector);
snow = new Snow(address(weth), buyFee, collector);
​
vm.deal(alice, 100 ether);
vm.prank(alice);
weth.deposit{value: 10 ether}();
vm.prank(alice);
weth.approve(address(snow), type(uint256).max);
}
​
function testExploit_DoublePayment() public {
uint256 requiredWeth = buyFee * PRECISION * amount; // = 1 ETH
uint256 sentEth = 2 ether;
​
uint256 aliceEthBefore = alice.balance;
uint256 aliceWethBefore = weth.balanceOf(alice);
uint256 aliceSnowBefore = snow.balanceOf(alice);
​
vm.prank(alice);
snow.buySnow{value: sentEth}(amount);
​
// 1. Alice got Snow
assertEq(snow.balanceOf(alice), aliceSnowBefore + amount);
​
// 2.Alice's ETH balance decreased by 1 sentEth
assertEq(alice.balance, aliceEthBefore - sentEth);
​
// 3.Alice's WETH balance decreased by the requiredWETH amount
assertEq(weth.balanceOf(alice), aliceWethBefore - requiredWeth);
​
// 4.The contract holds a sentEth of ETH (stuck)
assertEq(address(snow).balance, sentEth);
​
// 5.The contract retains the requiredWeth from WETH
assertEq(weth.balanceOf(address(snow)), requiredWeth);
​
// 6.calls for collectFee
vm.prank(collector);
snow.collectFee();
​
// 7.received WETH
assertEq(weth.balanceOf(collector), requiredWeth);
​
// 8. received ETH
assertEq(collector.balance, sentEth);
​
// 9. The contract no longer has any funds.
assertEq(address(snow).balance, 0);
assertEq(weth.balanceOf(address(snow)), 0);
​
// 10. Summary: Alice paid 2 ETH + 1 WETH (worth 1 ETH) and received Snow worth 1 ETH
// Net loss = 2 ETH
console.log("Alice net loss (ETH):", sentEth);
}
}
​

Recommended Mitigation

Refund any excess ETH sent by the user. If the user sends ETH but the amount is not exactly the required fee, return the ETH and then proceed with the WETH transfer (or revert entirely to avoid confusion).

Alternatively, you could revert the transaction entirely if the user sends ETH when they should use WETH, to avoid confusion. But the simplest fix is to return the ETH.

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);
- }
+ uint256 required = s_buyFee * amount;
+ if (msg.value == required) {
+ _mint(msg.sender, amount);
+ } else {
+ // Refund any ETH sent
+ if (msg.value > 0) {
+ payable(msg.sender).transfer(msg.value);
+ }
+ // Then pull WETH
+ i_weth.safeTransferFrom(msg.sender, address(this), required);
+ _mint(msg.sender, amount);
+ }
s_earnTimer = block.timestamp;
emit SnowBought(msg.sender, amount);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 7 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!