nobody2018 - Malicious PartyA/PartyB can prevent themselves from being liquidated

[sherlock-admin]

nobody2018

high

Malicious PartyA/PartyB can prevent themselves from being liquidated

Summary

Since the protocol is deployed on multiple chains, the tx on some chains is easy to be front-run. When liquidating PartyA, LibMuon.verifyPartyAUpnl(upnlSig, partyA) will be called to verify the parameter upnlSig from off-chain. This function internally uses the current partyANonces[partyA] to calculate the hash to verify the upnlSig. When this upnlSig is generated from off-chain, the partyANonces[partyA] used is obtained via [nonceOfPartyA](https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/ViewFacet.sol#L127-L129) at that time. In most cases, partyANonces[partyA] at these two moments are equal. However, a malicious PartyA can monitor the mempool and front-run LiquidationFacet.liquidatePartyA to make current partyANonces[partyA] different from the partyANonces[partyA] used when generating upnlSig, causing verifyPartyAUpnl to fail to verify upnlSig. In this way, the liquidatePartyA tx will revert. PartyB can also use a similar method to avoid being liquidated.

Vulnerability Detail

When a liquidator notices that PartyA can be liquidated, he will initiate liquidation against PartyA via LiquidationFacet.liquidatePartyA. The flow of this function is as follows:

LiquidationFacet.liquidatePartyA(partyA, upnlSig)
  LiquidationFacetImpl.liquidatePartyA(partyA, upnlSig)
    LibMuon.verifyPartyAUpnl(upnlSig, partyA)
    ...
  ...

Let's look at the code snippet of LibMuon.verifyPartyAUpnl:

File: symmio-core\contracts\libraries\LibMuon.sol
093:         bytes32 hash = keccak256(
094:             abi.encodePacked(
095:                 muonLayout.muonAppId,
096:                 upnlSig.reqId,
097:                 address(this),
098:                 partyA,
099:->               AccountStorage.layout().partyANonces[partyA],//this value can be changed.
100:                 upnlSig.upnl,
101:                 upnlSig.timestamp,
102:                 getChainId()
103:             )
104:         );
105:->       verifyTSSAndGateway(hash, upnlSig.sigs, upnlSig.gatewaySignature); //revert inside it due to wrong hash

Suppose the following scenario:

bob is PartyA, accountLayout.balances[bob]=1e18, accountLayout.allocatedBalances[bob]=1000018.

1. bob creates a quote via PartyAFacet.sendQuote.
2. As PartyB, alice locks the quote and opens the position.
3. As time goes by, bob can be liquidated.
4. As the liquidator, tom initiates liquidation against bob via LiquidationFacet.liquidatePartyA. This tx enters the mempool. Suppose partyANonces[bob] used when generating upnlSig is 10.
5. Bob's robot monitors this tx, and immediately initiates AccountFacet.allocate(1) to front-run it. At this time, partyANonces[bob] is increased by 1, equal to 11.
6. LiquidationFacet.liquidatePartyA will revert due to failure to verify upnlSig.

Why does AccountFacet.allocate(1) increase partyANonces[bob] by 1? Let's look at the code of this function:

File: symmio-core\contracts\facets\Account\AccountFacetImpl.sol
41:     function allocate(uint256 amount) internal {
......
49:->       accountLayout.partyANonces[msg.sender] += 1;
50:         accountLayout.balances[msg.sender] -= amount;
51:         accountLayout.allocatedBalances[msg.sender] += amount;
52:     }

As mentioned above, a malicious PartyA can always prevent itself from being liquidated in this way. Until the price of the symbol moves towards a profitable direction, PartyA finally makes a profit.

Similarly, a malicious PartyB can do the same. The flow of LiquidationFacet.liquidatePartyB is as follows:

LiquidationFacet.liquidatePartyB(partyB, partyA, upnlSig)
  LiquidationFacetImpl.liquidatePartyB(partyB, partyA, upnlSig)
    LibMuon.verifyPartyBUpnl(upnlSig, partyB, partyA)
    ...
  ...

A similar explanation for PartyB will no longer be made here. The relevant functions are as follows:

• [LiquidationFacetImpl.liquidatePartyB](https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L249)

• [LibMuon.verifyPartyBUpnl](https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/libraries/LibMuon.sol#L152)

• [AccountFacetImpl.allocateForPartyB](https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L128) and [AccountFacetImpl.transferAllocation](https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L101-L105)

Impact

As mentioned above, malicious PartyA or PartyB can make profits in the following situations:

1. When the price moves in profitable direction, he can close the position to realize a profit.
2. When the price moves in the direction of loss, they can prevent liquidation until the price moves in the opposite direction. ultimately achieve profitability.

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L23

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/libraries/LibMuon.sol#L99

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L49

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L249

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/libraries/LibMuon.sol#L152

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L128

Tool used

Manual Review

Recommendation

This issue does not appear to be easy to fix. A simple and effective suggestion is to set a minimum allocation amount to increase the attack cost.

Duplicate of #233