Failure to withdraw Ichi vault LP tokens to the user

Summary

A user is able to specify an amount amountLpWithdraw to withdraw ICHI vault LP tokens from their owned position directly to the user. However, due to a bug in the calculation of the amount of LP tokens to withdraw, the requested tokens are not transferred to the user and instead remain stuck in the spell contract.

Vulnerability Detail

The IchiVaultSpell.closePosition function first withdraws lpTakeAmt collateral (ICHI vault LP tokens) from the bank that was previously deposited when opening the position.

The closePosition function allows a user to specify an amount amountLpWithdraw of ICHI vault LP tokens to withdraw and to transfer back to the caller (user). This parameter is passed on to the IchiVaultSpell.withdrawInternal function.

In the IchiVaultSpell.withdrawInternal function, the amount of ICHI vault LP tokens to withdraw is calculated in step 2 by subtracting the specified amountLpWithdraw parameter from the current ICHI vault LP token balance (collateral that have been withdrawn previously from the bank - see IchiVaultSpell.closePosition#L355).

If a non-zero value is provided as amountLpWithdraw, the amount of ICHI vault LP tokens to withdraw from the ICHI vault will be correspondingly lower. The remaining ICHI vault LP tokens, which are not used to withdraw from the ICHI vault, remain in the spell contract and are not refunded to the caller.

It is worth noting that any other user or MEV bot can create a new ICHI vault spell position and withdraw those leftover ICHI vault LP tokens by closing the position.

In contrast, the Alpha Homora SushiswapSpellV1 implementation refunds (transfers) any LP tokens intended to be withdrawn by the user (amtLPWithdraw) in line 338.

Impact

ICHI vault LP tokens intended to be withdrawn and transferred to the user (caller) are not transferred to the user and remain stuck in the spell contract. The user lost those LP tokens.

Code Snippet

spell/IchiVaultSpell.closePosition

341: function closePosition(
342:     uint256 strategyId,
343:     address collToken,
344:     address borrowToken,
345:     uint256 lpTakeAmt,
346:     uint256 amountRepay,
347:     uint256 amountLpWithdraw,
348:     uint256 amountShareWithdraw
349: )
350:     external
351:     existingStrategy(strategyId)
352:     existingCollateral(strategyId, collToken)
353: {
354:     // 1. Take out collateral
355:     doTakeCollateral(strategies[strategyId].vault, lpTakeAmt);
356:
357:     withdrawInternal(
358:         strategyId,
359:         collToken,
360:         borrowToken,
361:         amountRepay,
362:         amountLpWithdraw,
363:         amountShareWithdraw
364:     );
365: }

spell/IchiVaultSpell.sol#L294-L295

276: function withdrawInternal(
277:     uint256 strategyId,
278:     address collToken,
279:     address borrowToken,
280:     uint256 amountRepay,
281:     uint256 amountLpWithdraw,
282:     uint256 amountShareWithdraw
283: ) internal {
284:     Strategy memory strategy = strategies[strategyId];
285:     IICHIVault vault = IICHIVault(strategy.vault);
286:     uint256 positionId = bank.POSITION_ID();
287:
288:     // 1. Compute repay amount if MAX_INT is supplied (max debt)
289:     if (amountRepay == type(uint256).max) {
290:         amountRepay = bank.borrowBalanceCurrent(positionId, borrowToken);
291:     }
292:
293:     // 2. Calculate actual amount to remove
294:     uint256 amtLPToRemove = vault.balanceOf(address(this)) -
295:         amountLpWithdraw;
296:
297:     // 3. Withdraw liquidity from ICHI vault
298:     vault.withdraw(amtLPToRemove, address(this));
299:
300:     // 4. Swap withdrawn tokens to initial deposit token
...      // [...]
318:
319:     // 5. Withdraw isolated collateral from Bank
320:     doWithdraw(collToken, amountShareWithdraw);
321:
322:     // 6. Repay
323:     doRepay(borrowToken, amountRepay);
324:
325:     _validateMaxLTV(strategyId);
326:
327:     // 7. Refund
328:     doRefund(borrowToken);
329:     doRefund(collToken);
330: }

Tool used

Manual Review

Recommendation

Consider refunding the remaining Ichi vault LP tokens using doRefund(address(vault)); at the end of the IchiVaultSpell.withdrawInternal function.

Duplicate of #151

sherlock-audit / 2023-02-blueberry-judging

berndartmueller - Failure to withdraw Ichi vault LP tokens to the user #322