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

M-04] `buySnow` keeps the ETH sent when it isn't the exact fee, and charges again in WETH

alvarok29

Root + Impact

Description

  • A buyer pays once, in the currency they choose: paying with ETH should not also charge WETH, and any excess ETH should not be retained.

  • buySnow selects the payment path with a strict equality on msg.value. If the value is not exactly the price, execution falls into the WETH branch: it charges the full price in WETH and keeps the ETH sent, without crediting or refunding it - the user pays roughly twice.

function buySnow(uint256 amount) external payable canFarmSnow {
@> if (msg.value == (s_buyFee * amount)) { // strict equality
_mint(msg.sender, amount);
} else {
@> i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount)); // full WETH...
_mint(msg.sender, amount); // ...and the ETH stays in the contract
}
}

Risk

Likelihood:

  • A user sends a msg.value that is off by any amount while holding WETH allowance - a rounding or UI mismatch is enough.

Impact:

  • Direct loss of user funds: the ETH sent is trapped in the contract, recoverable only by the collector.

  • The user effectively pays close to double the intended price.

Proof of Concept

The test has Alice send exactly 1 wei less than the fee while holding WETH allowance: she receives her SNOW, is charged the full 5 WETH, and additionally loses ~5 ETH that stays trapped in the contract.

Verified with Foundry (test_elEthEnviadoQueNoCuadraSePierde), forge test passing:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;
​
import {Test, console2} from "forge-std/Test.sol";
import {Snow} from "../src/Snow.sol";
import {DeploySnow} from "../script/DeploySnow.s.sol";
import {MockWETH} from "../src/mock/MockWETH.sol";
​
contract SnowFarmingYPagoTest is Test {
Snow snow;
DeploySnow deployer;
MockWETH weth;
uint256 FEE;
​
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address carol = makeAddr("carol");
address atacante = makeAddr("atacante");
​
function setUp() public {
deployer = new DeploySnow();
snow = deployer.run();
weth = deployer.weth();
FEE = deployer.FEE();
}
​
/// [M] `s_earnTimer` es UNA variable global, no un cooldown por usuario: en cuanto alguien
/// usa `earnSnow()`, TODOS los demas quedan bloqueados una semana aunque no hayan farmeado
/// nunca. El farming pasa a ser una carrera de uno contra todos.
function test_elCooldownEsGlobalYBloqueaAlRestoDeUsuarios() public {
vm.prank(alice);
snow.earnSnow();
assertEq(snow.balanceOf(alice), 1, "alice farmea");
​
// bob y carol no han farmeado JAMAS, y aun asi no pueden
vm.prank(bob);
vm.expectRevert(); // S__Timer
snow.earnSnow();
​
vm.prank(carol);
vm.expectRevert(); // S__Timer
snow.earnSnow();
​
assertEq(snow.balanceOf(bob), 0, "bob bloqueado por la accion de otro");
assertEq(snow.balanceOf(carol), 0, "carol tambien");
}
​
/// [M] Peor: `buySnow()` tambien escribe `s_earnTimer`, asi que cualquiera puede reiniciar
/// el reloj a voluntad y dejar `earnSnow()` inutilizable PARA SIEMPRE, comprando de vez en
/// cuando. El farming gratuito deja de existir para todo el mundo.
function test_unAtacantePuedeCongelarElFarmingIndefinidamente() public {
vm.deal(atacante, 100 ether);
​
for (uint256 semana = 1; semana <= 4; semana++) {
vm.warp(block.timestamp + 1 weeks + 1);
​
// el atacante compra 1 SNOW justo antes de que se abra la ventana
vm.prank(atacante);
snow.buySnow{value: FEE}(1);
​
// ...y con eso reinicia el reloj de TODOS
vm.prank(alice);
vm.expectRevert(); // S__Timer
snow.earnSnow();
}
​
assertEq(snow.balanceOf(alice), 0, "alice nunca consigue farmear");
console2.log("coste del bloqueo por semana (wei):", FEE);
}
​
/// [L/M] Si `msg.value` no es EXACTAMENTE el precio, el contrato cobra el total en WETH y
/// ademas se queda el ETH enviado, sin acreditarlo ni devolverlo. Un usuario que se pase o
/// se quede corto paga dos veces.
function test_elEthEnviadoQueNoCuadraSePierde() public {
uint256 precio = FEE; // 1 SNOW
weth.mint(alice, precio);
vm.deal(alice, precio);
​
uint256 ethAntes = alice.balance;
​
vm.startPrank(alice);
weth.approve(address(snow), precio);
// alice se queda corta por 1 wei: cae en la rama del WETH
snow.buySnow{value: precio - 1}(1);
vm.stopPrank();
​
assertEq(snow.balanceOf(alice), 1, "recibe su SNOW");
assertEq(weth.balanceOf(alice), 0, "ha pagado el precio COMPLETO en WETH");
assertEq(alice.balance, ethAntes - (precio - 1), "y encima ha perdido el ETH enviado");
assertEq(address(snow).balance, precio - 1, "el ETH se queda atrapado en el contrato");
​
console2.log("pagado en WETH:", precio);
console2.log("ETH perdido ademas:", precio - 1);
}
​
/// [M] SNOW declara 18 decimales (hereda de ERC20 sin sobrescribir `decimals()`), pero
/// `buySnow`/`earnSnow` mintean `amount` en CRUDO. Con `amount = 1` el usuario paga el
/// precio entero por 1 wei de token: 0,000000000000000001 SNOW.
function test_sePagaElPrecioEnteroPorUnWeiDeToken() public {
assertEq(snow.decimals(), 18, "el token declara 18 decimales");
​
vm.deal(alice, FEE);
vm.prank(alice);
snow.buySnow{value: FEE}(1);
​
assertEq(snow.balanceOf(alice), 1, "recibe 1 WEI de SNOW, no 1 SNOW");
assertLt(snow.balanceOf(alice), 1e18, "muy lejos de un token entero");
​
console2.log("ETH pagado: ", FEE);
console2.log("SNOW recibido (wei): ", snow.balanceOf(alice));
console2.log("hacen falta 1e18 wei para 1 SNOW entero -> coste real: FEE * 1e18");
}
}

Recommended Mitigation

Do not use a strict equality to pick the payment path, and refund any excess ETH instead of retaining it:

function buySnow(uint256 amount) external payable canFarmSnow {
uint256 price = s_buyFee * amount;
- if (msg.value == price) { _mint(msg.sender, amount); }
- else { i_weth.safeTransferFrom(msg.sender, address(this), price); _mint(msg.sender, amount); }
+ if (msg.value > 0) {
+ if (msg.value < price) revert S__InsufficientPayment();
+ if (msg.value > price) { (bool ok,) = payable(msg.sender).call{value: msg.value - price}(""); require(ok); }
+ } else {
+ i_weth.safeTransferFrom(msg.sender, address(this), price);
+ }
+ _mint(msg.sender, amount);
}
Updates

Lead Judging Commences

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