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

Payment Logic Flaw Creates Confusion Between ETH and WETH Payments

evmninja

Root + Impact

Description

The buySnow() function uses a strict equality check for ETH payments, causing unintuitive behavior when users send incorrect amounts. The function uses WETH instead of ETH in all cases where the exact ETH amount is not sent, even when ETH is provided.

The Snow contract allows users to purchase tokens using either ETH or WETH. However, the payment logic in the buySnow() function uses a strict equality check (==) rather than a greater-than-or-equal check (>=) when verifying ETH payments:

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);
}

This implementation creates two significant issues:

  1. If a user sends slightly more ETH than required, the contract will ignore the ETH payment and attempt to use WETH instead.

  2. If a user sends slightly less ETH than required, the contract will also attempt to use WETH rather than simply reverting with a clear message about insufficient ETH.

This behavior is unintuitive and confusing for users, as they might send ETH but the contract would deduct WETH from their balance. Additionally, excess ETH sent to the contract becomes trapped, as there is no mechanism to refund it.

Risk

Likelihood: High

  • Users commonly make small mistakes when entering payment amounts

  • Any user who sends more than the exact ETH amount will trigger this issue

  • Occurs during normal contract usage

Impact:

Impact: Medium

  • Users can lose funds by sending excess ETH that becomes trapped

  • Confusing payment behavior leads to unexpected WETH transfers

  • Poor user experience and potential fund loss

Proof of Concept

function test_POC_TrappedETH() public {
console2.log("=== POC 2: Trapped ETH Vulnerability ===");
uint256 excessAmount = 2 ether;
uint256 totalSent = FEE + excessAmount;
// Alice accidentally sends more ETH than required
vm.prank(alice);
snow.buySnow{value: totalSent}(1);
// BUG: The contract used WETH transfer instead of handling the ETH properly
// Alice sent ETH but contract deducted WETH due to != comparison
console2.log("BUG DEMONSTRATED: Alice sent ETH but contract used WETH");
console2.log("ETH was sent but WETH was deducted due to flawed logic");
}

Recommended Mitigation

1. Use >= for ETH comparisons and refund excess ETH:

function buySnow(uint256 amount) external payable canFarmSnow {
if (amount == 0) revert S__ZeroValue();
if (msg.value >= (s_buyFee * amount)) {
// Handle ETH payment and refund excess
uint256 excess = msg.value - (s_buyFee * amount);
if (excess > 0) {
(bool success,) = payable(msg.sender).call{value: excess}("");
require(success, "ETH refund failed");
}
_mint(msg.sender, amount);
} else if (msg.value > 0) {
// User sent some ETH but not enough
revert S__InsufficientETH(msg.value, s_buyFee * amount);
} else {
// Handle WETH payment when no ETH is sent
i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
_mint(msg.sender, amount);
}
​
emit SnowBought(msg.sender, amount);
}
​
// Add a new error for clarity
error S__InsufficientETH(uint256 sent, uint256 required);

This fix:

  1. Properly handles cases where users send excess ETH by refunding them

  2. Clearly distinguishes between ETH and WETH payment intentions

  3. Provides more informative errors when insufficient ETH is sent

  4. Prevents user confusion and potential fund loss

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.