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

Fund Loss due to Strict ETH Value Check

mohankrishkotte

Strict ETH Value Check in Snow.sol::buySnow() Causes Fund Loss

Description

The `Snow.sol::buySnow` function uses strict equality (==), leading to:
​
1. Permanent loss of excess ETH if msg.value > (s_buyFee * amount) or if msg.value < (s_buyFee * amount)
​
2. Unnecessary reverts when users accidentally send more or less ETH than needed
​
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:

Reason 1: This occurs when a user directly interacts with the contract (e.g., via Etherscan, a script, or wallet) and sends more ETH than required while intending to use the WETH payment path.

Reason 2: There's no strict validation enforcing msg.value == 0 in the WETH path, so the contract silently accepts unintended ETH.

Impact:

  1. Users lose ETH permanently, as it becomes irrecoverable unless the contract owner manually refunds or exposes a withdrawal mechanism.

  2. Increases contract balance without corresponding state accounting, making audits and fund tracking harder.

Proof of Concept :


This test shows how overpaying with ETH while having WETH approved causes funds to get stuck:

​
function test_ETHGetsStuckWhenOverpaying() public {
// Setup
address user = makeAddr("overpayingUser");
uint256 amount = 1;
uint256 requiredFee = snow.s_buyFee() * amount;
uint256 overpaymentAmount = requiredFee + 1 ether;
​
// Fund user with ETH and WETH
vm.deal(user, overpaymentAmount);
weth.mint(user, requiredFee);
​
// User approves WETH spending (normal WETH payment setup)
vm.prank(user);
weth.approve(address(snow), requiredFee);
​
// User accidentally sends ETH (overpayment) while having WETH approved
vm.prank(user);
snow.buySnow{value: overpaymentAmount}(amount);
​
// Verification
// 1. ETH got stuck in contract (full overpaymentAmount)
assertEq(address(snow).balance, overpaymentAmount, "ETH stuck in contract");
// 2. WETH was also transferred (double payment)
assertEq(weth.balanceOf(address(snow)), requiredFee, "WETH was taken");
assertEq(weth.balanceOf(user), 0, "User's WETH was spent");
​
// 3. User got their tokens
assertEq(snow.balanceOf(user), amount, "User received tokens");
// 4. User's ETH balance decreased by overpayment
assertEq(user.balance, 0, "User's ETH was taken");
}
​

Recommended Mitigation

The original logic checked msg.value == totalFee and defaulted to WETH transfer in all other cases — including cases where users overpaid in ETH, leading to silent acceptance of ETH and WETH being pulled, causing ETH to get stuck in the contract.

The updated logic mitigates this by:

  1. Validating ETH flow upfront:

    • If any ETH is sent, it must exactly match the expected amount (totalFee), or the transaction reverts.

    • This eliminates the accidental case of overpayment going unnoticed.

  2. Using else for WETH path only:

    • The logic now clearly separates ETH and WETH branches based on whether ETH was sent at all.

  3. Adding nonReentrant:

    • While not directly related to this bug, this protects the mint function from reentrancy via ERC777 or malicious tokens.

This ensures that:

  • ETH is only accepted intentionally and correctly.

  • WETH is only pulled when no ETH is sent.

  • No asset is accidentally stuck or double-charged.

- 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);
- }
+ function buySnow(uint256 amount) external payable canFarmSnow nonReentrant {
+ require(amount > 0, "Amount must be > 0");
+ uint256 totalFee = s_buyFee * amount;
+
+ if (msg.value > 0) {
+ require(msg.value == totalFee, "Incorrect ETH sent");
+ } else {
+ i_weth.safeTransferFrom(msg.sender, address(this), totalFee);
+ }
+
+ _mint(msg.sender, amount);
+ s_earnTimer = block.timestamp;
+
+ emit SnowBought(msg.sender, amount);
+ }
​
Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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