0x52 - ShortLongSpell#openPosition can cause user unexpected liquidation when increasing position size

[sherlock-admin]

0x52

high

ShortLongSpell#openPosition can cause user unexpected liquidation when increasing position size

Summary

When increasing a position, all collateral is sent to the user rather than being kept in the position. This can cause serious issues because this collateral keeps the user from being liquidated. It may unexpectedly leave the user on the brink of liquidation where a small change in price leads to their liquidation.

Vulnerability Detail

ShortLongSpell.sol#L129-L141

    {
        IBank.Position memory pos = bank.getCurrentPositionInfo();
        address posCollToken = pos.collToken;
        uint256 collSize = pos.collateralSize;
        address burnToken = address(ISoftVault(strategy.vault).uToken());
        if (collSize > 0) {
            if (posCollToken != address(wrapper))
                revert Errors.INCORRECT_COLTOKEN(posCollToken);
            bank.takeCollateral(collSize);
            wrapper.burn(burnToken, collSize);
            _doRefund(burnToken);
        }
    }

In the above lines we can see that all collateral is burned and the user is sent the underlying tokens. This is problematic as it sends all the collateral to the user, leaving the position collateralized by only the isolated collateral.

Best case the user's transaction reverts but worst case they will be liquidated almost immediately.

Impact

Unfair liquidation for users

Code Snippet

ShortLongSpell.sol#L111-L151

Tool used

Manual Review

Recommendation

Don't burn the collateral

[Gornutz]

https://github.com/Blueberryfi/blueberry-core/commit/78a0779538f9c89106a5d540ae0e72c2db5a7096

[IAm0x52]

Fix looks good. Contract no longer burns previous position (since it was unnecessary anyways)