LP tokens are stucked and not sent back to withdrawing user from the IchiSpell contract
Summary
When users withdraw their assets from IchiSpell.sol, the function unwinds their position and sends them back their assets, but it never sends them back the amount they requested to withdraw, leaving the tokens stuck in the Spell contract.
Vulnerability Detail
When a user withdraws from IchiVaultSpell.sol, they either call closePosition() or closePositionFarm(), both of which make an internal call to _withdraw().
Different arguments are passed into the function via the param
In order to accomplish these goals, the contract does the following...
Removes the LP tokens from the ERC1155 holding them for collateral.
Calculates the number of LP tokens to withdraw from the vault.
Converts the non-borrowed token that was withdrawn in the borrowed token .
Withdraw the underlying token from Compound.
Pay back the borrowed token to Compound.
Validate that this situation does not put us above the maxLTV for our loans.
Sends the remaining borrow token that weren't paid back and withdrawn underlying tokens to the user.
doRefund(borrowToken);
doRefund(collToken);
Most importantly, the step of sending the remaining LP tokens to the user is skipped, even though the function specifically does the calculations to ensure that aamountPosRemove is held back from being taken out of the vault.
Impact
Users who close their positions and choose to keep LP tokens (rather than unwinding the position for the constituent tokens) will have their LP tokens stuck permanently in the IchiSpell contract.
Bauchibred
high
LP tokens are stucked and not sent back to withdrawing user from the IchiSpell contract
Summary
When users withdraw their assets from
IchiSpell.sol,
the function unwinds their position and sends them back their assets, but it never sends them back the amount they requested to withdraw, leaving the tokens stuck in the Spell contract.Vulnerability Detail
When a user withdraws from IchiVaultSpell.sol, they either call
closePosition()
orclosePositionFarm()
, both of which make an internal call to_withdraw()
.Different arguments are passed into the function via the
param
In order to accomplish these goals, the contract does the following...
Removes the LP tokens from the ERC1155 holding them for collateral.
Calculates the number of LP tokens to withdraw from the vault.
Converts the non-borrowed token that was withdrawn in the borrowed token .
Withdraw the underlying token from Compound.
Pay back the borrowed token to Compound.
Validate that this situation does not put us above the maxLTV for our loans.
Sends the remaining borrow token that weren't paid back and withdrawn underlying tokens to the user. doRefund(borrowToken); doRefund(collToken);
Most importantly, the step of sending the remaining LP tokens to the user is skipped, even though the function specifically does the calculations to ensure that aamountPosRemove is held back from being taken out of the vault.
Impact
Users who close their positions and choose to keep LP tokens (rather than unwinding the position for the constituent tokens) will have their LP tokens stuck permanently in the IchiSpell contract.
Code Snippet
https://github.com/sherlock-audit/2023-04-blueberry/blob/96eb1829571dc46e1a387985bd56989702c5e1dc/blueberry-core/contracts/spell/IchiSpell.sol#L174-L236
Tool used
Manual Review
Recommendation
Add an additional line to the withdrawInternal() function to refund all LP tokens as well: