This repository has been archived by the owner on May 26, 2023. It is now read-only.
obront - Withdrawals from IchiVaultSpell have no slippage protection so can be frontrun, stealing all user funds #130
Labels
Has Duplicates
A valid issue with 1+ other issues describing the same vulnerability
Medium
A valid Medium severity issue
Reward
A payout will be made for this issue
Sponsor Confirmed
The sponsor acknowledged this issue is valid
Will Fix
The sponsor confirmed this issue will be fixed
obront
high
Withdrawals from IchiVaultSpell have no slippage protection so can be frontrun, stealing all user funds
Summary
When a user withdraws their position through the
IchiVaultSpell
, part of the unwinding process is to trade one of the released tokens for the other, so the borrow can be returned. This trade is done on Uniswap V3. The parameters are set in such a way that there is no slippage protection, so any MEV bot could see this transaction, aggressively sandwich attack it, and steal the majority of the user's funds.Vulnerability Detail
Users who have used the
IchiVaultSpell
to take positions in Ichi will eventually choose to withdraw their funds. They can do this by callingclosePosition()
orclosePositionFarm()
, both of which call towithdrawInternal()
, which follows loosely the following logic:The issue exists in the swap, where Uniswap is called with the following function:
The 4th variable is called
sqrtPriceLimitX96
and it represents the square root of the lowest or highest price that you are willing to perform the trade at. In this case, we've hardcoded in that we are willing to take the worst possible rate (highest price in the event we are trading 1 => 0; lowest price in the event we are trading 0 => 1).The
IchiVaultSpell.sol#uniswapV3SwapCallback()
function doesn't enforce any additional checks. It simply sends whatever delta is requested directly to Uniswap.While it is true that there is an
amountRepay
parameter that is inputted by the user, it is not sufficient to protect users. Many users will want to make only a small repayment (or no repayment) while unwinding their position, and thus this variable will only act as slippage protection in the cases where users intend to repay all of their returned funds.With this knowledge, a malicious MEV bot could watch for these transactions in the mempool. When it sees such a transaction, it could perform a "sandwich attack", trading massively in the same direction as the trade in advance of it to push the price out of whack, and then trading back after us, so that they end up pocketing a profit at our expense.
Because many of the ICHI token pairs have small amounts of liquidity (for example, ICHI-WBTC has under $350k), such an attack could feasible take the majority of the funds, leaving the user with close to nothing. See more details on liquidity here: https://info.uniswap.org/#/tokens/0x111111517e4929d3dcbdfa7cce55d30d4b6bc4d6
Impact
Users withdrawing their funds through the
IchiVaultSpell
who do not plan to repay all of the tokens returned from Uniswap could be sandwich attacked, losing their funds by receiving very little of their borrowed token back from the swap.Code Snippet
https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/spell/IchiVaultSpell.sol#L300-L317
https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/spell/IchiVaultSpell.sol#L407-L442
Tool used
Manual Review
Recommendation
Have the user input a slippage parameter to ensure that the amount of borrowed token they receive back from Uniswap is in line with what they expect.
Alternatively, use the existing oracle system to estimate a fair price and use that value in the
swap()
call.The text was updated successfully, but these errors were encountered: