simon135 - Threw the design of liquidations in the system, PartyA will be able to get out liquidations in `partyAAvailableBalanceForLiquidation->unpl`

[sherlock-admin]

simon135

high

Threw the design of liquidations in the system, PartyA will be able to get out liquidations in partyAAvailableBalanceForLiquidation->unpl

Summary

So looking at how positions are handled in this system it should liquidate a bad position but not cover up the bad positions so they don't get liquidated but there is a bug in the way this is handled

Vulnerability Detail

Here is an example of how it's not supposed to work and why it should work like how I recommend ex: Let's say PartyA->allocated =0(disregarding allocated), so unpl for position[1]=15 andposition[2] = -15sobalance = 0` and liquidations won't happen but it should be liquidated ( explain why down below) but PartyA can leverage the other side of the same position and get out of loss and won't be able to liquidated. How this differs from a neutral position is that if one position gets a very negative pnl and one is a very positive pnl it should be liquidated because then we can take the profits from the other position but in this system, liquidations won't happen. https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/libraries/LibAccount.sol#L144

   int256 freeBalance = int256(accountLayout.allocatedBalances[partyA]) -
            int256(accountLayout.lockedBalances[partyA].cva + accountLayout.lockedBalances[partyA].lf);
            // Here the pnl and lockedBalances cover not just one position but the global accounting for the system 
        return freeBalance + upnl;

The pnl is PartyA liquidation will be total pnl which makes this issue possible https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L23 In other protocols, the position that is doing badly will get liquidated like ex: past certain point the protocol will say that your position cant hold with your leverage and we will liquidate you but in this protocol,it wont see that is happening and instead it shows that their no bad position available for liquidation

Impact

The main impact is that this shouldn't be possible because an attacker can take advantage of this and make risky positions of the same token and not get liquidated and get off the risk of trading. So negative pnl won't be counted as negative pnl so the Attacker can open up many positions without the interference of negative pnl. Other similar protocols have a global state that checks those negative positions. U won't get the negative effects of a bad position and it won't get liquidated means no loss for an attacker just keeps positions in limbo openState and no push even if hurts PartyB.It's a negative push on PartyB balance instead.

Code Snippet

showed above more of a design issue than something wrong with the code

Tool used

Forge Manual Review

Recommendation

Redisgn this part of the system to account for bad positions and keep global state that can be checked or be more harsh on PartyA for positions Have a global account layer and the root cause is that it doesn't liquidate bad positions like other protocols