LP tokens might get left in the Spell contract and be taken by someone else

Summary

LP tokens can get left in the spell contract, enabling someone else to take them.

Vulnerability Detail

The function IchiVaultSpell.closePosition has a parameter amountLpWithdraw. Contrary to its name, the only thing it does is to actually reduce the amount of LP tokens to withdraw from the IchiVault:

uint256 amtLPToRemove = vault.balanceOf(address(this)) -
    amountLpWithdraw;

After clarification with the sponsor, the name of this parameter was meant to be amountLpToLeave. Regardless of the naming, the problem is that the amounts left in the contract are not handled anywhere currently, so this would cause the LP to remain in the contract until IchiVaultSpell.closePosition gets called with amountLpWithdraw == 0 to withdraw the full balance of the Spell contract. This would allow anyone to take the LPs left behind by someone else.

Impact

Theft of funds. Severity deemed to be medium due to the requisites of faulty integration/incorrect parameter usage.

Code Snippet

https://github.com/sherlock-audit/2023-02-blueberry/blob/main/contracts/spell/IchiVaultSpell.sol#L294-L298

Tool used

Manual Review

Recommendation

Omit the parameter amountLpWithdraw if it is obsolete. Otherwise handle the leftover funds.

Duplicate of #151

sherlock-audit / 2023-02-blueberry-judging

minhtrng - LP tokens might get left in the Spell contract and be taken by someone else #351