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

Snow::buySnow selects its payment rail by strict equality on msg.value and never refunds, so an inexact ETH amount is kept while the full price is also charged in WETH

crisscholte

buySnow selects its payment rail by strict equality on msg.value and never refunds, so any inexact ETH amount is kept by the contract while the full price is also taken in WETH

Description

  • buySnow is documented as accepting either native ETH or WETH. A payable function that accepts a native payment normally either refunds any excess or reverts when the amount is wrong, so that a buyer cannot pay more than the quoted price.

  • buySnow has no parameter expressing which rail the caller intends to use. It infers the intent from a strict equality test on msg.value, and treats any value other than the exact price as a WETH purchase. The else branch pulls the full price in WETH and mints, but nothing ever returns the ETH the caller attached. The contract is payable, so that ETH is simply retained. A buyer who is wrong by a single wei pays the full price twice, once in ETH and once in WETH, and there is no path to recover the ETH leg.

function buySnow(uint256 amount) external payable canFarmSnow {
@> if (msg.value == (s_buyFee * amount)) { // strict equality is the only way to select the ETH rail
_mint(msg.sender, amount);
} else {
@> i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount)); // full price taken again
_mint(msg.sender, amount);
}
// @> no refund of msg.value anywhere in this function, on either branch
​
s_earnTimer = block.timestamp;
​
emit SnowBought(msg.sender, amount);
}

The retained ETH is not stuck in the contract. collectFee forwards address(this).balance to the fee collector, so the overpayment is silently converted into protocol revenue rather than being returned to the buyer.

There is a second, wider path to the same loss that needs no WETH and no approval at all. With amount of zero, the WETH leg becomes a zero-value safeTransferFrom, which succeeds regardless of allowance, so the call does not revert. Any ETH attached is kept and nothing is minted:

// amount = 0, msg.value = 1 ether
// msg.value (1 ether) != s_buyFee * 0 (0) -> else branch
// safeTransferFrom(caller, contract, 0) -> succeeds with zero allowance
// _mint(caller, 0) -> mints nothing
// msg.value -> retained

Risk

Likelihood:

  • Only one exact value selects the ETH rail. Every other value routes to WETH, so the function punishes the ordinary defensive habit of attaching a small margin to a payable call, which most contracts refund.

  • The price is s_buyFee * amount where s_buyFee is 5e18 for the deployment in script/DeploySnow.s.sol, so the exact figure a buyer must attach is a large non-obvious number computed from a private-looking scaling factor. Getting it wrong is the expected outcome of a manual interaction rather than an unusual one.

  • Wallets commonly hold a standing max approval to a contract they have used before, which is the state in which the double charge occurs.

  • The zero-amount variant requires no WETH balance and no approval whatsoever, so it applies to every caller.

Impact:

  • Direct loss of user funds. The buyer pays the full quoted price twice for a single purchase, once in ETH and once in WETH, and receives only one purchase worth of Snow.

  • The loss is unbounded by the protocol: it is whatever the caller attached, and nothing in the contract caps or returns it.

  • The lost ETH is transferred to the fee collector by collectFee, so it becomes protocol revenue taken from a user who never agreed to pay it.

  • In the zero-amount case the caller receives nothing at all in exchange.

Proof of Concept

Snow is deployed here directly with the project's own FEE = 5 from script/DeploySnow.s.sol, because Snow exposes no getter for its WETH address and the deploy script wires an instance the Helper does not return.

Part 1 - off by one wei, charged twice. The test closes with the counterfactual: the same purchase with the exact amount is charged once, so the double charge is caused by the inexact value and not by the fixture.

function test_C5_inexact_eth_is_kept_and_weth_is_charged_as_well() public {
uint256 price = snow.s_buyFee() * 1; // price of 1 base unit of Snow
​
weth.mint(alice, price);
vm.prank(alice);
weth.approve(address(snow), type(uint256).max);
vm.deal(alice, price + 1);
​
uint256 ethBefore = alice.balance;
uint256 wethBefore = weth.balanceOf(alice);
​
// Alice intends to pay in ETH but is off by a single wei.
vm.prank(alice);
snow.buySnow{value: price + 1}(1);
​
uint256 ethSpent = ethBefore - alice.balance;
uint256 wethSpent = wethBefore - weth.balanceOf(alice);
​
assertEq(snow.balanceOf(alice), 1, "she received 1 base unit of Snow");
assertEq(ethSpent, price + 1, "her ETH was taken");
assertEq(wethSpent, price, "and the full price was taken again in WETH");
assertEq(address(snow).balance, price + 1, "the ETH sits in the contract");
​
console2.log("ETH taken (wei):", ethSpent);
console2.log("WETH taken (wei):", wethSpent);
console2.log("Snow received :", snow.balanceOf(alice));
​
// COUNTERFACTUAL: with the exact amount, only ETH is taken.
weth.mint(bob, price);
vm.prank(bob);
weth.approve(address(snow), type(uint256).max);
vm.deal(bob, price);
uint256 bobWethBefore = weth.balanceOf(bob);
​
vm.prank(bob);
snow.buySnow{value: price}(1);
​
assertEq(weth.balanceOf(bob), bobWethBefore, "exact payer is charged once");
}

Part 2 - ETH kept with no WETH and no approval.

function test_C5b_eth_is_kept_with_no_weth_and_no_approval() public {
assertEq(weth.balanceOf(alice), 0, "alice holds no WETH");
assertEq(weth.allowance(alice, address(snow)), 0, "and has approved nothing");
​
vm.deal(alice, 1 ether);
​
// msg.value (1 ether) != s_buyFee * 0 (0), so the else branch runs.
// safeTransferFrom of ZERO tokens succeeds without allowance, so the
// call does not revert, mints nothing, and keeps the ETH.
vm.prank(alice);
snow.buySnow{value: 1 ether}(0);
​
assertEq(alice.balance, 0, "alice's ETH is gone");
assertEq(snow.balanceOf(alice), 0, "she received no Snow at all");
assertEq(address(snow).balance, 1 ether, "the contract kept it");
​
console2.log("ETH lost with no WETH and no approval (wei):", address(snow).balance);
console2.log("Snow received :", snow.balanceOf(alice));
}

Results:

[PASS] test_C5_inexact_eth_is_kept_and_weth_is_charged_as_well()
ETH taken (wei): 5000000000000000001
WETH taken (wei): 5000000000000000000
Snow received : 1
​
[PASS] test_C5b_eth_is_kept_with_no_weth_and_no_approval()
ETH lost with no WETH and no approval (wei): 1000000000000000000
Snow received : 0

In part 1 the buyer paid 5.000000000000000001 ETH plus 5 WETH, a little over ten ETH of value, for a single base unit of Snow priced at five.

Scope note, stated rather than left for a judge to find: this is not attacker-triggerable. msg.value is chosen by the caller, s_buyFee is fixed at construction and has no setter, so no third party can manoeuvre a victim into the wrong branch. The finding is that the contract converts an ordinary user mistake into an uncapped, unrecoverable loss, on a payable function where refunding is the near-universal convention.

Recommended Mitigation

Take the payment rail as an explicit parameter rather than inferring it, and reject any ETH that is not exactly the price. Refunding is also acceptable; rejecting is simpler and leaves no partial-payment state.

+ error S__IncorrectPayment();
​
- function buySnow(uint256 amount) external payable canFarmSnow {
+ function buySnow(uint256 amount, bool payWithEth) external payable canFarmSnow {
+ if (amount == 0) {
+ revert S__ZeroValue();
+ }
+ uint256 price = s_buyFee * amount;
+
+ if (payWithEth) {
+ if (msg.value != price) {
+ revert S__IncorrectPayment();
+ }
+ } else {
+ if (msg.value != 0) {
+ revert S__IncorrectPayment();
+ }
+ i_weth.safeTransferFrom(msg.sender, address(this), price);
+ }
+
+ _mint(msg.sender, amount);
- 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);
}

If the external signature must be preserved, keep the inference but refund the difference on the WETH branch and reject a nonzero remainder on the ETH branch, so that no path can retain ETH the buyer did not owe.

Distinctness

This is a payment-handling defect in buySnow and is independent of the previously reported issues. It is separate from the report on s_earnTimer, which concerns the same function only insofar as buySnow writes that shared timer: that report's fix removes the timer write and changes nothing about payment, and this report's fix corrects the payment handling and changes nothing about the timer. The two touch the same function for unrelated reasons and require different changes.

Updates

Lead Judging Commences

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