Snowman Merkle Airdrop

AI First Flight #10
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: high
Likelihood: high
Invalid

High Bug Report: Excess ETH Lost in buySnow()

# Summary
The `buySnow()` function has a critical accounting flaw. When a user sends ETH but the amount doesn't exactly match the cost, the contract takes the WETH path — but the excess ETH sent is **never refunded**. Additionally, there's no validation preventing ETH from being sent alongside the WETH transfer, meaning users can lose both their excess ETH AND pay WETH on top.
---
## Vulnerable Code
```solidity
// Snow.sol:79-90
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);
}
```
---
## Impact
- **User loses excess ETH:** If `msg.value > cost`, the excess ETH stays in the contract with no refund mechanism.
- **Double payment possible:** The else branch doesn't check `msg.value == 0`. A user can send excess ETH AND have WETH pulled from them simultaneously.
- **Funds locked forever:** The excess ETH accumulates in the contract. Only `collectFee()` can transfer it out, but that sends to the collector, not the original sender.
- **No user protection:** Users who accidentally overpay lose their funds permanently.
---
## Proof of Concept
```solidity
// Scenario 1: User sends excess ETH
// Cost for 10 Snow = 10 * s_buyFee = 1 ETH
// User accidentally sends 1.5 ETH
uint256 cost = 1 ether; // s_buyFee * amount
uint256 sent = 1.5 ether;
snow.buySnow{value: 1.5 ether}(10);
// Result:
// - msg.value (1.5 ETH) != cost (1 ETH) → enters else branch
// - 1 WETH pulled from user via safeTransferFrom
// - 10 Snow minted to user
// - BUT: 1.5 ETH stays in contract (user loses 1.5 ETH + pays 1 WETH)
// - Net loss: 1.5 ETH (stuck) + 1 WETH (fee) = 2.5 ETH equivalent
// Scenario 2: User sends exact ETH but has WETH approved
// User sends 1 ETH exactly
snow.buySnow{value: 1 ether}(10);
// Result:
// - msg.value == cost → enters if branch (uses ETH, not WETH)
// - 10 Snow minted
// - Correct behavior (but only by luck of exact match)
// Scenario 3: User sends 0 ETH, has WETH
snow.buySnow{value: 0}(10);
// Result:
// - msg.value (0) != cost → enters else branch
// - WETH pulled from user
// - Correct behavior
```
**Worst case attack/griefing:**
1. Attacker front-runs a user's `buySnow()` transaction
2. This is less about attack, more about protocol losing user funds due to poor design
3. Any rounding error or UI imprecision causes permanent loss
---
## Recommended Fix
```solidity
function buySnow(uint256 amount) external payable canFarmSnow {
uint256 cost = s_buyFee * amount;
if (msg.value >= cost) {
// Accept ETH, refund excess
_mint(msg.sender, amount);
if (msg.value > cost) {
payable(msg.sender).transfer(msg.value - cost);
}
} else {
// WETH path - reject any ETH sent
require(msg.value == 0, "Do not send ETH for WETH purchase");
i_weth.safeTransferFrom(msg.sender, address(this), cost);
_mint(msg.sender, amount);
}
emit SnowBought(msg.sender, amount);
}
```
Key changes:
1. Refund excess ETH when `msg.value > cost`
2. Reject ETH in the WETH path to prevent double payment
3. Use `>=` instead of `==` to handle overpayment gracefully
---
## References
- [Solidity Security: Forcing Ether](https://consensys.github.io/smart-contract-best-practices/attacks/forcing-ether/)
- [Checks-Effects-Interactions Pattern](https://docs.soliditylang.org/en/v0.8.24/security-considerations.html)
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours 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!