Snowman Merkle Airdrop

First Flight #42
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: medium
Likelihood: low
Invalid

[M-4] Malicious WETH address in Snow can allow user to buy token for free

[M-4] Malicious WETH address in Snow can allow user to buy token for free

Description

  • The Snow::buySnow() calls external functions from i_weth contract. The safeTransferFrom() function is a function from OpenZeppelin's SafeErc20 interface and internally it calls the transferFrom() function of the ERC20 token contract.

  • A malicious contract inheriting from ERC20 contract but overriding the transferFrom() function can contain attacker's own logic and return true even if no tranfer has been performed

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

Risk

Likelihood:

  • Deployer of contract puts malicious contract address either deliberately or due to human errors

Impact:

  • User can call buySnow() once with the amount needed for buying, but then the safeTransferFrom() function does nothing

Proof of Concept

Add the following testcase to the Snow.t.sol test suite:

function test_buyForFree() 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 is the malicious contract:

// 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 transferFrom(address, address, uint256) public override returns (bool) {
return true;
}
}

Recommended Mitigation

Check for the balance of the contract before and after the transfer has been done, and add the check to ensure that transfer has been done successfully

+ uint256 before = i_weth.balanceOf(msg.sender);
- 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);
+ uint256 after = i_weth.balanceOf(msg.sender);
+ assertEq(after, before - (s_buyFee * amount));
Updates

Lead Judging Commences

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.