Summary

Performing a forced bid during a liquidation event is the same as performing a forced bid to exit a position. But when a forced bid created during a liquidation event that partially exits a short results in a healthy short, if the short has had loseCollateral set to true they will still lose their collateral even though their position would no longer be liquidatable. This is handled differently if a user is the one to exit a position however and if they only partially exit and make their short position healthy again by doing so the flag is removed from their position and they are no longer liquidatable. Given that in both scenarios a user's position is made healthy again by a partial closing they should be allowed to maintain their position for both.

Vulnerability Details

In MarginCallPrimaryFacet::liquidate there's a call to _performForcedBid which calls BidOrdersFacet::createForcedBid, the resulting bid is either fully or partially matched with the sell orders on the OB and liquidate then calls _fullorPartialLiquidation which adds the short to the TAPP's shortRecord if the MarginCallPrimary struct has had loseCollateral set to true:

note that loseCollateral can be set to true if the TAPP doesn't have sufficient ethEscrowed to cover the ethDebt of the position:

MarginCallPrimaryFacet::_performForcedBid

so if the TAPP isn't seeded with an initial amount of ethEscrowed or if the TAPP hasn't performed many liquidations and has only a small ethEscrowed balance users will always lose their collateral.

If the user were to call exitShort instead there's an equivalent call to createForcedBid in ExitShortFacet::exitShort:

but if the bid order is only partially filled there's a check that resets the flag if the position is no longer liquidatable:

ExitShortFacet::exitShort

Impact

This results in users whose positions are now healthy being unfairly liquidated and losing their collateral. The TAPP as a result accumulates healthy positions as well as unhealthy positions.

Tools Used

Manual Review

Recommendations

Add a check of the resulting CR after partially liquidating the position to see if it is above the minimum and only take a user's collateral if it is: