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

No check if transfer fail in `collectFee`

rubik0n

Description

When the transfer fail the ERC20 will return a boolean and will not revert in i_weth.transfer(s_collector, collection);

because of this the function will continue to execute without transfering.

// In the contract we use ERC20 implementation that will return boolean
function transfer(address to, uint256 value) external returns (bool);

Risk

Likelihood: Low

  • ERC20 transfer failures are not a frequent occurrence, so the likehood is low.

Impact: High

  • Fail to transfer WETH to collector

Proof of Concept

To prove this I will make the required modifications in few places.

  1. We need to modify a bit the MockWETH.sol for the purpose of the test

contract MockWETH is ERC20 {
// >>> CONSTRUCTOR
constructor() ERC20("MockWETH", "mWETH") {}
​
// >>> FUNCTION
function mint(address receiver, uint256 amount) external {
_mint(receiver, amount);
}
​
function transfer(address, uint256) public pure override returns (bool) {
return false; //@> simulate transfer failure
}
}
​

  1. We execute the current testCollectFee with little modification to assert only WETH balances after execution and swap values

function testCollectFee() public {
vm.startPrank(jerry);
weth.approve(address(snow), FEE);
snow.buySnow(1);
vm.stopPrank();
​
vm.prank(victory);
snow.buySnow{value: FEE}(1);
​
vm.prank(collector);
snow.collectFee();
​
​
assert(weth.balanceOf(collector) == 0); //@> Changed from `FEE` to `0`
assert(weth.balanceOf(address(snow)) == FEE); //@> Changed from `0` to `FEE`
​
}

As we can see the function executes without error and the WETH transfer fails silently.

Recommended Mitigation

Adding error S_TransferFail(); and checking if the transfer is failing.

function collectFee() external onlyCollector {
uint256 collection = i_weth.balanceOf(address(this));
- i_weth.transfer(s_collector, collection);
+ bool success = i_weth.transfer(s_collector, collection);
+ if (!success) {
+ revert S_TransferFail();
+ }
(bool collected,) = payable(s_collector).call{value: address(this).balance}("");
require(collected, "Fee collection failed!!!");
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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