This repository has been archived by the owner on May 26, 2023. It is now read-only.
lemonmon - TradingUtils::_executeTrade
will leak ETH to WETH
#98
Labels
lemonmon
high
TradingUtils::_executeTrade
will leak ETH to WETHSummary
If sellToken is ETH, and using Uniswap for the dex, and it is exact out trade, too much is deposited to the WETH and does not withdraw the excess amount. It will give wrong
amountSold
value as well as accounting error.Vulnerability Detail
trade.sellToken
is ETH and using Uniswap as dex, WETH should be used instead of ETH as Uniswap does not support ETH. There for TradingUtils wraps the ETH to WETH before trading.If the trade would be exact out, the amount
trade.limit
will be deposited to WETH instead of thetrade.amount
. However, because it is exact out, not all ETH deposited will be traded. In the current implementation, there is no logic to recover the excess deposit.As the
TradingUtils::_executeInternal
, which uses theTradingUtils::_executeTrade
will calculate theamountSold
based on the balance of ETH, it will return thetrade.limit
as theamountSold
, thus resulting in accounting error.Note: in the current implementation, the trade using Uniswap with ETH as sellToken would not even work, because the WETH is not properly approved (issue 2). This issue assumes that the issue is resolved.
Impact
amountSold
will reflect not the amount really sold, rather thetrade.limit
. It is unclear whether the excess amount of ETH, which is deposited for WETH can be recovered.Code Snippet
https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingUtils.sol?plain=1#L118-L137
https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingUtils.sol?plain=1#L29-L64
Tool used
Manual Review
Recommendation
In the
_executeTrade
, if the sellToken is ETH and it is exact out trade, recover excess deposit.The text was updated successfully, but these errors were encountered: