Snowman Merkle Airdrop

First Flight #42
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: medium
Invalid

Improper Handling of ETH and WETH Payments Leads to Locked Funds

Root + Impact

  • The intended behavior of the buySnow function is to allow users to purchase S tokens using one of two mutually exclusive payment methods: either by sending the exact required amount of native ETH with the transaction, or by having the contract pull the required amount of WETH tokens via transferFrom.

  • The contract's logic incorrectly conflates these two payment paths. If a user attempts to pay with ETH but sends an amount that does not exactly match the required price, the function's else block is executed. If that user also has a sufficient WETH allowance approved for the contract, the transaction will succeed using their WETH, but the native ETH sent in the msg.value becomes permanently locked in the contract with no mechanism for withdrawal, resulting in a direct loss of funds for the user.

// Snow.sol
function buySnow(uint256 amount)
// @> 1. The function is `payable`, allowing it to receive native ETH,
// which is the entry point for the issue.
external
payable
canFarmSnow
{
// @> 2. This strict equality check is the trigger. If a user sends ETH
// (msg.value > 0) but the amount is off by even 1 wei, this condition is false.
if (msg.value == (s_buyFee * amount)) {
_mint(msg.sender, amount);
} else {
// @> 3. Execution incorrectly falls here when msg.value is non-zero but not equal to the price.
// This block proceeds with the WETH transfer, completely ignoring the ETH sent in `msg.value`,
// thereby locking it in the contract forever.
i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
_mint(msg.sender, amount);
}
s_earnTimer = block.timestamp;
emit SnowBought(msg.sender, amount);
}

Risk

The risk is assessed as Medium. This rating is based on a Medium likelihood of occurrence combined with a High impact on the affected user.

Likelihood: Medium

  • Reason 1: This fund loss occurs when a user interacts directly with the contract (e.g., via Etherscan or a custom script) and makes a manual calculation error, sending an ETH amount that is not precisely equal to the calculated purchase price. Such "fat-finger" errors are common during direct contract interactions.

  • Reason 2: The vulnerability is triggered during interactions with a poorly implemented or buggy frontend. A user-facing application that miscalculates the required msg.value due to rounding errors, incorrect price data, or other bugs will cause its users to lose funds, even when they follow the intended user flow correctly.

Impact: High

  • Impact 1: The primary impact is a direct and permanent financial loss for the user. The ETH sent with the transaction becomes irrecoverably trapped within the contract's balance, as there is no function for withdrawal or refund by any party, including the contract owner.

  • Impact 2: The secondary impact is significant reputational damage to the project. Users who lose funds due to this predictable design flaw will lose trust in the protocol's safety and competence, leading to negative community sentiment and deterring future potential users from interacting with the ecosystem.

Proof of Concept

Explanation of the Proof of Concept

This PoC simulates a realistic user error scenario to prove the vulnerability:

  1. Objective: To show that a user attempting to buy Snow tokens can have their ETH locked in the contract if they send an incorrect amount while also having an active WETH approval.

  2. Setup:

    • A MockWETH contract is created to simulate WETH token functionality.

    • A user is given a balance of 10 WETH.

    • The user approves the Snow contract to spend 1 WETH. This is a critical precondition for the bug to manifest.

  3. Execution:

    • The user intends to buy 1 Snow token, which costs 1 WETH (or 1 ETH).

    • The user makes a mistake and sends only 0.5 ETH with the transaction instead of the required 1 ETH.

    • They call the buySnow(1) function with msg.value = 0.5 ether.

  4. Expected Malicious Outcome:

    • The if (msg.value == ...) check fails.

    • The else block executes, successfully transferring 1 WETH from the user.

    • The user receives 1 Snow token.

    • The 0.5 ETH sent by the user remains trapped in the Snow contract.

  5. Success Criteria: The test succeeds by asserting that the Snow contract's ETH balance is now 0.5 ETH, proving the funds are locked.


Proof of Concept Code (Foundry)

You can save this code as test/SnowFundLock.t.sol in a Foundry project.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
import {Test, console} from "forge-std/Test.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {Snow} from "../src/Snow.sol"; // Adjust path if needed
/**
* @title MockWETH Contract
* @notice A simple ERC20 mock for testing purposes.
*/
contract MockWETH is ERC20 {
constructor() ERC20("Wrapped Ether", "WETH") {}
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}
/**
* @title PoC: Improper ETH/WETH Handling Leads to Locked Funds
* @author Your Name/Handle
*/
contract SnowFundLockPoC is Test {
// === Contracts ===
Snow private snow;
MockWETH private weth;
// === Actors ===
address private owner = makeAddr("owner");
address private collector = makeAddr("collector");
address private user = makeAddr("user");
function setUp() public {
// Deploy Mock WETH and Snow contracts
weth = new MockWETH();
vm.prank(owner);
snow = new Snow(address(weth), 1 ether, collector); // Set buy fee to 1 ether
// Fund the user with WETH and have them approve the Snow contract
weth.mint(user, 10 ether);
vm.prank(user);
weth.approve(address(snow), 1 ether); // User approves spending of 1 WETH
// Label addresses for clarity
vm.label(owner, "Contract Owner");
vm.label(collector, "Fee Collector");
vm.label(user, "User (Victim)");
}
/**
* @notice This PoC demonstrates that sending an incorrect amount of ETH
* while having a WETH approval results in the sent ETH being locked.
*/
function test_PoC_EthIsLockedOnIncorrectPayment() public {
console.log("--- PoC: Fund Lock Vulnerability in buySnow ---");
// --- Step 1: Log initial state ---
uint256 initialUserWethBalance = weth.balanceOf(user);
uint256 initialContractEthBalance = address(snow).balance;
console.log("Initial User WETH Balance: %s ether", initialUserWethBalance / 1e18);
console.log("Initial Snow Contract ETH Balance: %s ether", initialContractEthBalance / 1e18);
assertEq(initialContractEthBalance, 0, "Contract should start with 0 ETH");
// --- Step 2: User attempts to buy 1 Snow token, but sends the wrong ETH amount ---
uint256 amountToBuy = 1;
uint256 incorrectEthSent = 0.5 ether; // User error: sends 0.5 ETH instead of 1 ETH
console.log("\nUser calls buySnow(%s) with msg.value = %s ether...", amountToBuy, incorrectEthSent / 1e18);
vm.prank(user);
// The transaction is executed with the incorrect msg.value
snow.buySnow{value: incorrectEthSent}(amountToBuy);
console.log("Transaction successful, but with unintended consequences.");
// --- Step 3: Assert the final state to prove the vulnerability ---
console.log("\n--- Verifying Final State ---");
// ASSERT 1 (CRITICAL): The incorrect ETH amount is now locked in the Snow contract.
uint256 finalContractEthBalance = address(snow).balance;
assertEq(finalContractEthBalance, incorrectEthSent, "FAIL: ETH was not locked in the contract!");
console.log("✅ SUCCESS: Snow contract ETH balance is now %s ether. Funds are locked.", finalContractEthBalance / 1e18);
// ASSERT 2: The user was charged in WETH instead.
uint256 finalUserWethBalance = weth.balanceOf(user);
uint256 wethSpent = initialUserWethBalance - finalUserWethBalance;
assertEq(wethSpent, 1 ether, "FAIL: Incorrect WETH amount was spent!");
console.log("✅ SUCCESS: User's WETH balance decreased by %s ether.", wethSpent / 1e18);
// ASSERT 3: The user still received the Snow token.
uint256 userSnowBalance = snow.balanceOf(user);
assertEq(userSnowBalance, amountToBuy, "FAIL: User did not receive the Snow token!");
console.log("✅ SUCCESS: User received %s Snow token(s).", userSnowBalance);
console.log("\nPoC successful: The user was charged WETH and their incorrectly sent ETH is permanently trapped.");
}
}

Recommended Mitigation

The vulnerability should be resolved by explicitly separating the payment logic for native ETH and WETH tokens. The contract must never assume a payment method; it must validate one or the other but never both simultaneously.

The recommended approach is to check if msg.value is greater than zero. If it is, the function should treat the transaction exclusively as an ETH payment and validate the amount accordingly. If msg.value is zero, it should proceed as a WETH payment. This creates two distinct and mutually exclusive execution paths, eliminating the ambiguity that leads to locked funds.

Step-by-Step Implementation

  1. Restructure the buySnow function's logic.
    Instead of an if-else block based on the result of a strict equality check, use the presence of msg.value to determine the payment type.

    // Snow.sol
    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 requiredPayment = s_buyFee * amount;
    +
    + if (msg.value > 0) {
    + // ETH Payment Path
    + require(msg.value == requiredPayment, "S__IncorrectEthAmount");
    + } else {
    + // WETH Payment Path
    + i_weth.safeTransferFrom(msg.sender, address(this), requiredPayment);
    + }
    +
    + // Mint tokens after successful payment
    + _mint(msg.sender, amount);
    s_userLastEarnTimestamp[msg.sender] = block.timestamp; // Assuming prior DoS fix is applied
    emit SnowBought(msg.sender, amount);
    }
  2. (Optional but Recommended) Add a new custom error.
    For better clarity and gas efficiency, define a custom error for incorrect ETH payments.

    // Snow.sol
    contract Snow is ERC20, Ownable {
    // ...
    error S__NotAllowed();
    error S__ZeroAddress();
    error S__ZeroValue();
    error S__Timer();
    error S__SnowFarmingOver();
    + error S__IncorrectEthAmount();
    // ...
    }

Corrected Code Summary

Here is the final, secure version of the buySnow function after applying the mitigation.

// Snow.sol
// ... (Add `error S__IncorrectEthAmount();` at the contract level)
function buySnow(uint256 amount) external payable canFarmSnow {
uint256 requiredPayment = s_buyFee * amount;
// Explicitly check for an ETH payment first.
if (msg.value > 0) {
// If ETH is sent, it MUST be the exact amount.
require(msg.value == requiredPayment, "S__IncorrectEthAmount");
} else {
// If no ETH is sent, proceed with the WETH transfer.
i_weth.safeTransferFrom(msg.sender, address(this), requiredPayment);
}
// This code is only reached if one of the payment paths succeeded.
_mint(msg.sender, amount);
// Assuming the DoS vulnerability has also been fixed using a mapping.
s_userLastEarnTimestamp[msg.sender] = block.timestamp;
emit SnowBought(msg.sender, amount);
}

Benefits of this Mitigation

  • Eliminates Fund Loss: It is now impossible for a user to send an incorrect ETH amount and also be charged WETH. The transaction will simply revert if the ETH value is wrong, protecting the user's funds.

  • Clear and Unambiguous Logic: The code's intent is now explicit. It clearly separates the two payment methods, reducing complexity and the chance of future bugs.

  • Improved User Experience: By failing early with a clear error message (S__IncorrectEthAmount), the contract provides immediate feedback to users or frontends about why a transaction failed, allowing them to correct the issue.

Updates

Lead Judging Commences

yeahchibyke Lead Judge
19 days ago
yeahchibyke Lead Judge 18 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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