Kose - Malicious PartyB can force their PartyA's into liquidation

[sherlock-admin]

Kose

medium

Malicious PartyB can force their PartyA's into liquidation

Summary

PartyB's have too many privileges and system is running around their actions and choices. Using these privileges, if partyB sees that their partyA is on the road to liquidation but trying to get rid of: they can block their partyA's request by simply locking new quotes and not accepting other requests.

Vulnerability Detail

In Symmetrical, the control over funds are mostly within the hands of partyB. Although this is the intented choice, it actually can create opportunities for partyB's to use them maliciously. Let's examine one scenario:

1. PartyA's funds are losing value and hence he/she is at the risk of liquidation.
2. PartyA wants to save their funds. The ways to prevent liquidity are:
   1. Closing position (via requestToClosePosition function)
   2. Opening hedge positions (via sendQuote function)
   3. Depositing and/or allocating new funds

Assuming user can not use option 3, now I will show how partyB can maliciously act to put obstacles into the option 1 and 2.

• Closing Position: After partyA make their closing request requestToClosePosition everything is in the hands of partyB of that specific position. If partyB don't call fillCloseRequest and close the position, partyA have no way to close the position without forcing. Let's look at forcing in this case: forceClosePosition

  function forceClosePosition(uint256 quoteId, PairUpnlAndPriceSig memory upnlSig) internal {
      AccountStorage.Layout storage accountLayout = AccountStorage.layout();
      MAStorage.Layout storage maLayout = MAStorage.layout();
      Quote storage quote = QuoteStorage.layout().quotes[quoteId];
  
      uint256 filledAmount = quote.quantityToClose;
      require(quote.quoteStatus == QuoteStatus.CLOSE_PENDING, "PartyAFacet: Invalid state");
      require(
          block.timestamp > quote.modifyTimestamp + maLayout.forceCloseCooldown,
          "PartyAFacet: Cooldown not reached"
      );
  ...

  As we can see, to forceClosePosition, partyA needs to wait until maLayout.forceCloseCooldown amount of time has passed. Which is set to (3000000000000000) in default. Which is practically impossible. But even if this cooldown changes, it won't solve the problem because again it might be too late to get rid of liquidation.

• Opening hedge position In order to neutralize their upcoming upnl's, partyA might decide opening new position via sendQuote. But here is the catch: Malicious partyB can lock this quote and refuse to open the position. If this happens, partyA will obviously want to call requestToCancelQuote which again the outcome is completely in the hands of partyB who is malicious in our case. If they want to force it, they again need to wait until the cooldown pass. Again even if the protocol reduce the cooldown to reasonable time, it doesn't change the fact that partyB has all the control and partyA can be liquidated during this cooldown time.

Although you might think that then this malicious behaviour gets punished, it actually won't happen because partyB has do nothing wrong in this scenario, he/she act according to his/her rights and it seems completely ok to the protocol to lock quotes without opening it, and prevent closing positions with this manner. Although whitelisting partyB's while sending quote might solve this issue, since this is not the default action in sendQuote we can obviously assume that this scenario is more than likely considering liquidations of partyA, leads to gain for partyB's.

Impact

PartyB's have too many privileges and can use them maliciously without doing anything malicious according to protocol. Using these privileges partyB's can prevent their partyA's new actions and forcefully liquidate them if they are on the dangerous zone (close to liquidation) in order to make profit.

Tool used

Manual Review

Code Snippets

Provided above

Recommendation

I would like to especially point out that this issue can not be solved by just changing forceCloseCooldown parameter. In markets everything can happen so fast and even choosing 1 hour period for this parameter again does not prevent malicious partyB's to reach their goals. One possible solution to this situation:

• Letting other partyB's than quote's partyB to be able to fillCloseRequest. Of course this might not go well with protocols main goals but it can be implemented such that both partyB make profit via this action. It will be better than partyA's force liquidations.
• But of course this problem might require more brainstorming between protocols developers, and I don't think there is an easy solution that won't hurt anyone. But we can safely say that solutions won't hurt as much as current status.

Duplicate of #224

[kosedogus]

Escalate

This issue is not a duplicate of #254. As I pointed out in recommendation section, changing forceCloseCooldown is not a solution to the problem provided above. What this issue is trying to show is Malicious partyB can prevent their partyA's to take any action and forcefully liquidate them as showed above. This issue is not just about force closing, it is also about maliciously locking quote's without opening (without even opening intention) them in order to make partyA liquidateable. One possible solution to this is provided above. I would like this issue to carefully reconsidered.

[sherlock-admin2]

Escalate

This issue is not a duplicate of #254. As I pointed out in recommendation section, changing forceCloseCooldown is not a solution to the problem provided above. What this issue is trying to show is Malicious partyB can prevent their partyA's to take any action and forcefully liquidate them as showed above. This issue is not just above force closing, it is also about maliciously locking quote's without opening (without even opening intention) them in order to make partyA liquidateable. One possible solution to this is provided above. I would like this issue to carefully reconsidered.

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 254, but do you think it's a valid issue on it's own?

[hrishibhat]

Result: Medium Duplicate of #224 This is a valid duplicate of #224

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• kosedogus: accepted