sherlock-audit / 2024-09-symmio-v0-8-4-update-contest-judging

0 stars 0 forks source link

xiaoming90 - `settleUpnl` function can be DOSed by other PartyBs/hedgers #39

Open sherlock-admin3 opened 1 week ago

sherlock-admin3 commented 1 week ago

xiaoming90

High

settleUpnl function can be DOSed by other PartyBs/hedgers

Summary

settleUpnl function can be DOSed. Users will not be able to close their positions, which could lead to a loss to the users due to the inability to close their positions. In the report's example, PartyA wants to close its losing position to stop its account from incurring further losses due to unfavorable market conditions. However, since PartyA is unable to close its losing position, PartyA continues to incur more losses and might eventually be liquidated. Furthermore, trading is a time-sensitive activity. Thus, any blockage or delay would eventually lead to a loss of assets.

Root Cause

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Assume the following scenario:

PartyA wants to close its losing position to stop its account from incurring further losses due to unfavorable market conditions.

However, the close position transaction cannot go through due to an insufficient allocated balance on PartyA's account. This is a similar scenario described in the Symm IO v0.8.4 documentation.

In order for PartyB_1 to close PartyA's position, it first has to execute the settleUpnl function to settle PartyA's unrealized profit so that PartyA's allocated balance will be "refilled" with more funds. Once there are sufficient funds in PartyA's allocated balance, PartyB_1 can proceed to fulfill PartyA's closing request.

The only option to increase the PartyA's allocated balance is to close the PartyA's winning position with PartyB_2.

When the settleUpnl function is executed, it will verify the signature provided by Muon via the verifySettlement function below. Line 20 below shows that the encoded data in the signature includes the nonce of the PartyBs of the positions/quotes where the PnL is to be settled. In this case, PartyB_2's nonce will be included in the signature.

Assume that the time when PartyB_1 fetches the signature is T0. At T0, PartyB_2's nonce is 890. Thus, the signature is signed against encoded data where PartyB_2's nonce is 890.

At T1, PartyB_1 executes the settleUpnl function along with the signature. However, PartyB_2 could front-run the transaction and perform certain actions (e.g., open/close position, charge funding rate) that will increment its own nonce to 891. In this case, the nonce in PartyB_1's signature and PartyB_2's nonce will differ, and the settleUpnl transaction will revert. PartyB_2 could repeat this continuously DOS or block PartyB_1 attempts to settle UPNL and close PartyA's positions.

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/muon/LibMuonSettlement.sol#L12

File: LibMuonSettlement.sol
12:     function verifySettlement(SettlementSig memory settleSig, address partyA) internal view {
13:         MuonStorage.Layout storage muonLayout = MuonStorage.layout();
14:         // == SignatureCheck( ==
15:         require(block.timestamp <= settleSig.timestamp + muonLayout.upnlValidTime, "LibMuon: Expired signature");
16:         // == ) ==
17:         bytes memory encodedData;
18:         uint256[] memory nonces = new uint256[](settleSig.quotesSettlementsData.length);
19:         for (uint8 i = 0; i < settleSig.quotesSettlementsData.length; i++) {
20:             nonces[i] = AccountStorage.layout().partyBNonces[QuoteStorage.layout().quotes[settleSig.quotesSettlementsData[i].quoteId].partyB][partyA];
21:             encodedData = abi.encodePacked(
22:                 encodedData,  // Append the previously encoded data
23:                 settleSig.quotesSettlementsData[i].quoteId,
24:                 settleSig.quotesSettlementsData[i].currentPrice,
25:                 settleSig.quotesSettlementsData[i].partyBUpnlIndex
26:             );
27:         }
28:         bytes32 hash = keccak256(
29:             abi.encodePacked(
30:                 muonLayout.muonAppId,
31:                 settleSig.reqId,
32:                 address(this),
33:                 "verifySettlement",
34:                 nonces,
35:                 AccountStorage.layout().partyANonces[partyA],
..SNIP..
44:     }

Impact

Users will not be able to close their positions, which could lead to a loss to the users due to the inability to close their positions. In the above example, PartyA wants to close its losing position to stop its account from incurring further losses due to unfavorable market conditions. However, since PartyA is unable to close its losing position, PartyA continues to incur more losses and might eventually be liquidated. Furthermore, trading is a time-sensitive activity. Thus, any blockage or delay would eventually lead to a loss of assets.

PoC

No response

Mitigation

No response