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

User can lost ETH if send incorrect amount

soldev

Root + Impact

Description

  • If the user sends ETH (msg.value > 0), but the amount does not exactly match s_buyFee * amount, the function enters the else branch.

  • In the else branch, the function does not use the sent ETH at all. Instead, it tries to take WETH from the user via safeTransferFrom.

  • The ETH sent by the user remains in the contract, and is not refunded. This means the user loses their ETH, and also pays in WETH for the same purchase.

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:

User accidentally sends 1.1 ETH instead of 1 ETH (maybe due to a UI bug or miscalculation). The function enters the else branch, takes WETH from the user, and mints tokens. The 1.1 ETH is stuck in the contract (unless there is a withdraw function for the owner), and the user is overcharged.

Proof of Concept

The buySnow function checks if msg.value == s_buyFee * amount (which is 1 ETH). Since 1.1 ETH ≠ 1 ETH, it enters the else branch.
The contract does not refund the 1.1 ETH, and also requires the user to pay in WETH.
The 1.1 ETH is lost (stuck in the contract), and the user is double-charged.
Only collector can return it to user.

Recommended Mitigation

We could require(msg.value == 0 || msg.value == s_buyFee * amount) at the top, and revert if the ETH sent is not exactly what is expected.

Or we can refund the difference.

// only correct amount
function buySnow(uint256 amount) external payable canFarmSnow {
uint256 totalFee = s_buyFee * amount;
if (msg.value > 0) {
require(msg.value == totalFee, "Incorrect ETH sent");
_mint(msg.sender, amount);
} else {
i_weth.safeTransferFrom(msg.sender, address(this), totalFee);
_mint(msg.sender, amount);
}
s_earnTimer = block.timestamp;
emit SnowBought(msg.sender, amount);
}
​
// or we can refund the difference
function buySnow(uint256 amount) external payable canFarmSnow {
uint256 totalFee = s_buyFee * amount;
if (msg.value > 0) {
require(msg.value >= totalFee, "Not enough ETH sent");
_mint(msg.sender, amount);
// Refund any excess ETH
if (msg.value > totalFee) {
(bool success, ) = msg.sender.call{value: msg.value - totalFee}("");
require(success, "Refund failed");
}
} 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.