Withdrawals from IchiVaultSpell have no slippage protection so can be frontrun, stealing all user funds
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.\\nUsers who have used the `IchiVaultSpell` to take positions in Ichi will eventually choose to withdraw their funds. They can do this by calling `closePosition()` or `closePositionFarm()`, both of which call to `withdrawInternal()`, which follows loosely the following logic:\\nsends the LP tokens back to the Ichi vault for the two underlying tokens (one of which was what was borrowed)\\nswaps the non-borrowed token for the borrowed token on UniV3, to ensure we will be able to pay the loan back\\nwithdraw our underlying token from the Compound fork\\nrepay the borrow token loan to the Compound fork\\nvalidate that we are still under the maxLTV for our strategy\\nsend the funds (borrow token and underlying token) back to the user\\nThe issue exists in the swap, where Uniswap is called with the following function:\\n```\\nif (amountToSwap > 0) {\\n    swapPool = IUniswapV3Pool(vault.pool());\\n    swapPool.swap(\\n        address(this),\\n        !isTokenA,\\n        int256(amountToSwap),\\n        isTokenA\\n            ? UniV3WrappedLibMockup.MAX_SQRT_RATIO - 1 \\n            : UniV3WrappedLibMockup.MIN_SQRT_RATIO + 1, \\n        abi.encode(address(this))\\n    );\\n}\\n```\\n\\nThe 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).\\nThe `IchiVaultSpell.sol#uniswapV3SwapCallback()` function doesn't enforce any additional checks. It simply sends whatever delta is requested directly to Uniswap.\\n```\\nfunction uniswapV3SwapCallback(\\n    int256 amount0Delta,\\n    int256 amount1Delta,\\n    bytes calldata data\\n) external override {\\n    if (msg.sender != address(swapPool)) revert NOT_FROM_UNIV3(msg.sender);\\n    address payer = abi.decode(data, (address));\\n\\n    if (amount0Delta > 0) {\\n        if (payer == address(this)) {\\n            IERC20Upgradeable(swapPool.token0()).safeTransfer(\\n                msg.sender,\\n                uint256(amount0Delta)\\n            );\\n        } else {\\n            IERC20Upgradeable(swapPool.token0()).safeTransferFrom(\\n                payer,\\n                msg.sender,\\n                uint256(amount0Delta)\\n            );\\n        }\\n    } else if (amount1Delta > 0) {\\n        if (payer == address(this)) {\\n            IERC20Upgradeable(swapPool.token1()).safeTransfer(\\n                msg.sender,\\n                uint256(amount1Delta)\\n            );\\n        } else {\\n            IERC20Upgradeable(swapPool.token1()).safeTransferFrom(\\n                payer,\\n                msg.sender,\\n                uint256(amount1Delta)\\n            );\\n        }\\n    }\\n}\\n```\\n\\nWhile 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.\\nWith 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.
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.\\nAlternatively, use the existing oracle system to estimate a fair price and use that value in the `swap()` call.
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.
```\\nif (amountToSwap > 0) {\\n    swapPool = IUniswapV3Pool(vault.pool());\\n    swapPool.swap(\\n        address(this),\\n        !isTokenA,\\n        int256(amountToSwap),\\n        isTokenA\\n            ? UniV3WrappedLibMockup.MAX_SQRT_RATIO - 1 \\n            : UniV3WrappedLibMockup.MIN_SQRT_RATIO + 1, \\n        abi.encode(address(this))\\n    );\\n}\\n```\\n