sherlock-audit / 2022-09-notional-judging

4 stars 2 forks source link

lemonmon - `TradingUtils::_executeInternal` fail to approve WETH #97

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

lemonmon

medium

TradingUtils::_executeInternal fail to approve WETH

Summary

Upon using Uniswap for dex, and sellToken is ETH, WETH should be approved to Uniswap. However, it is not approved, therefore the trade will fail and gas spent for this transaction will be lost.

Vulnerability Detail

Uniswap does not support ETH transfer, so the trading should be done by WETH.

If the sellToken is ETH, the uniswap adapter sets the spender as uniswap router, so it is not zero address. Thus, in the TradingUtils::_executeInternal (line 45 TradingUtils.sol), it calls _approve. The TradingUtils::_approve function (line 115) calls TokenUtils::checkApprove, which will just return if the token address is zero. In this case, the sellToken is ETH so the _approve will just return. So, at the end no WETH was approved to uniswap.

In the parameters to the uniswap, it is selling WETH, thus without the approval the trade will fail.

Impact

Any attempt to use Uniswap for selling ETH will fail. Users who tries to use it will lose gas and should use some other dex.

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingUtils.sol?plain=1#L43-L46

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingUtils.sol?plain=1#L113-L116

FILE: TokenUtils.sol

 18     function checkApprove(IERC20 token, address spender, uint256 amount) internal {
 19         if (address(token) == address(0)) return;
 20
 21         IEIP20NonStandard(address(token)).approve(spender, amount);
 22         _checkReturnCode();
 23     }

Tool used

Manual Review

Recommendation

For uniswap, when sellToken is ETH, approve.