lemonmon - `TradingUtils::_executeTrade` will withdraw/deposit the whole balance

[sherlock-admin]

lemonmon

high

TradingUtils::_executeTrade will withdraw/deposit the whole balance

Summary

If sellToken is not WETH or ETH, the local variable preTradeBalance in TradingUtils::_executeTrade remains to be the default value, which is zero.
If buyToken is WETH or ETH, it will deposit or withdraw the whole balance.
This will result in accounting error of the balance. It will also return the whole balance as amountBought, which will be used in other part of contracts.

Vulnerability Detail

TradingUtils::_executeTrade has a local variable preTradeBalance (line 125). The value is not set when the variable is defined, so it starts with zero. If trade.sellToken is either WETH or ETH (and when the spender's conditions is met), the preTradeBAlance will be set. But if the trade.sellToken is none of WETH or ETH, the preTradeBalance will remain to be zero.

If the trade.buyToken is WETH or ETH, the post trade ETH or WETH balance will be compared to the preTradeBalance. Unless trade.sellToken is WETH or ETH, the preTradeBalance will be zero. Since the current balance will be likely bigger than zero, the condition of post trade balance to be bigger than zero will be likely to be met. (line 143 and 152 of TradingUtils.sol)

As the result, the whole balance of ETH will be deposited (if the buyToken is WETH), or the whole balance of WETH will be withdrwan (if the buyToken is ETH). It causes accounting error, as unexpected amount of ETH is withdrawn or deposited..

Moreover, the TradingUtils::_executeInternal, which uses TradingUtils::_executeTrade, will use the balance of WETH or ETH to determine amountBought and return the value as if it will really bought by the trade. So, the returning value amountBought will be the whole balance of WETH or ETH.

For example, TwoTokenPoolUtils::_deposit in the line 191, uses the returned amountBought result to _joinPoolAndStake.

Impact

• Serious accounting error, as well as amountBought can be more than actually bought amount

Code Snippet

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

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 TradingUtils::_executeTrade set the preTradeBalance, if the buyToken is WETH or ETH.

Duplicate of #110