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

[M-3] Snow::buySnow() not checking for return value from token transfer

soarinskysagar

[M-3] Snow::buySnow() not checking for return value from token transfer

Description

  • In Snow::buySnow() we are transferring tokens from caller to the contract. SafeERC20's safeTransferFrom() function is being used to do the operation

  • safeTransferFrom() internally calls the transferFrom() function. Even if the tx fails and returns false, the function does not check for the result and continues the operation. The transaction may be forced to fail deliberately so that the caller can get Snow tokens for free

@> i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
_mint(msg.sender, amount);

Risk

Likelihood:

  • Transaction fails

  • Malicious WETH contract

  • Deliberate tx fails

Impact:

  • Snow tokens can be bought for free

Proof of Concept

Add the the following testcase to the test suite of Snow:

function test_buyFreeToken() public {
vm.startPrank(ashley);
TestWETH test_weth = new TestWETH();
uint256 fee = 1;
​
Snow newSnow = new Snow(address(test_weth), fee, collector);
newSnow.buySnow(10);
​
uint256 res = newSnow.balanceOf(ashley);
assertEq(res, 100);
}

Also add the following contract to the import list. This contract is just for replicating a transfer call that fails and does not do any transfers.

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity 0.8.24;
​
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {Snow} from "../../src/Snow.sol";
​
contract TestWETH is ERC20 {
​
uint256 public counter;
address public immutable owner;
address public snow_addr;
​
constructor() ERC20("MockWETH", "mWETH") {
owner = msg.sender;
}
​
function transfer(address, uint256) public pure override returns (bool) {
return true;
}
}

Recommended Mitigation

Add checks to ensure that the transfer function returns a true before continuing to execute the rest of the function

- i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
+ bool res = i_weth.safeTransferFrom(msg.sender, address(this), (s_buyFee * amount));
+ assert(res == true);
_mint(msg.sender, amount);
Updates

Lead Judging Commences

yeahchibyke Lead Judge
5 months ago
yeahchibyke Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.