simon135 - Both Partys can make `CloseQuote` revert by deallocate their funds and allowing the closing/liquidations to go threw

[sherlock-admin]

simon135

high

Both Partys can make CloseQuote revert by deallocate their funds and allowing the closing/liquidations to go threw

Summary

When an actor wants to deallocate or allocate there is no check if their position is healthy or not before closeQuote

Vulnerability Detail

3 scenarios 1.PartyB allocates and then deallocates before the force closing of the position and they get out that loss, PartyA won't be able to close their position besides liquidation but PartyB won't have funds to even liquidate allocate https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L41

ex: PartyB needs 2500 tokens to make this position not liquidatable, so he allocates that much and then deallocates most of his funds and since that happens below the code will revert https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/libraries/LibQuote.sol#L176 as you can see since if remove the allocated, the Upnl will revert causing force close not to happen and PartyA funds to be stuck and sit in the protocol The only other requirement is that Upnl would have to be more than the funds left allocatedBalances over after PartyB deallocated

     LibMuon.verifyPartyBUpnl(upnlSig, msg.sender, partyA);
        int256 availableBalance = LibAccount.partyBAvailableForQuote(
            upnlSig.upnl,
            msg.sender,
            partyA
        );

2. Here PartyA does the same thing but waits until Pnl is in his favor and gains free profit
3. In emergency mode PartyB can try to close PartyA's position but PartyA deallocates which would cause a Break in the assumed spec and plus PartyA won't be able to be liquidated/position won't be able to be emergency closed

   Impact

   as said above with the right risky position and large enough pnl with smaller partys' it will cause reverts which would affect how those positions are handled which can make those funds stuck/free profit

   Code Snippet

   function deallocateForPartyB(
       uint256 amount,
       address partyA,
       SingleUpnlSig memory upnlSig
   ) internal {
       AccountStorage.Layout storage accountLayout = AccountStorage.layout();
       require(
           accountLayout.partyBAllocatedBalances[msg.sender][partyA] >= amount,
           "PartyBFacet: Insufficient locked balance"
       );
       LibMuon.verifyPartyBUpnl(upnlSig, msg.sender, partyA);
       int256 availableBalance = LibAccount.partyBAvailableForQuote(
           upnlSig.upnl,
           msg.sender,
           partyA
       );

   closeQuote

      if (hasMadeProfit) {
            accountLayout.allocatedBalances[quote.partyA] += pnl;
            accountLayout.partyBAllocatedBalances[quote.partyB][quote.partyA] -= pnl;
        } else {
            accountLayout.allocatedBalances[quote.partyA] -= pnl;  
            accountLayout.partyBAllocatedBalances[quote.partyB][quote.partyA] += pnl;
        }

Tool used

Forge Manual Review

Recommendation

The main issue here is that the protocol allows for some deallocating before an important function with no solvent function checking after what the state of the actor is at

Add a solvent function to allocate and deallocate so that PartyA/PartyB can't get out of any commitment/quotes like after the deallocating check: PartyAForLiquidation>0 passes

Duplicate of #241

[snn20]

escalate for 10 My issue is dealing with how deallocate and allocate can be abused in different ways

82 is dealing with PartyB making a profit during PartyA liquidation and then causing some issue

I hope my report can be deduped and reread and then determined if valid.

[sherlock-admin2]

escalate for 10 My issue is dealing with how deallocate and allocate can be abused in different ways

82 is dealing with PartyB making a profit during PartyA liquidation and then causing some issue

I hope my report can be deduped and reread and then determined if valid.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Evert0x]

@ctf-sec I agree this is not a duplicate of 82, what are your thoughts? Is this a valid issue on it's own?

[hrishibhat]

@snn20 I think this issue can be a duplicate of #241

[hrishibhat]

Result: Medium Duplicate of #241 The underlying issue seems to be the same as that of #241

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• snn20: accepted