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
lemonmon
high
TradingUtils::_executeTrade
will withdraw/deposit the whole balanceSummary
If sellToken is not WETH or ETH, the local variable
preTradeBalance
inTradingUtils::_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 asamountBought
, which will be used in other part of contracts.Vulnerability Detail
TradingUtils::_executeTrade
has a local variablepreTradeBalance
(line 125). The value is not set when the variable is defined, so it starts with zero. Iftrade.sellToken
is either WETH or ETH (and when the spender's conditions is met), thepreTradeBAlance
will be set. But if thetrade.sellToken
is none of WETH or ETH, thepreTradeBalance
will remain to be zero.If the
trade.buyToken
is WETH or ETH, the post trade ETH or WETH balance will be compared to thepreTradeBalance
. Unlesstrade.sellToken
is WETH or ETH, thepreTradeBalance
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 usesTradingUtils::_executeTrade
, will use the balance of WETH or ETH to determineamountBought
and return the value as if it will really bought by the trade. So, the returning valueamountBought
will be the whole balance of WETH or ETH.For example,
TwoTokenPoolUtils::_deposit
in the line 191, uses the returnedamountBought
result to_joinPoolAndStake
.Impact
amountBought
can be more than actually bought amountCode 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 thepreTradeBalance
, if the buyToken is WETH or ETH.Duplicate of #110