xiaoming90 - An excessive number of user positions can be settled even if the shortfall allocated balance needed for fulfilling the close request is only a small amount

[sherlock-admin3]

xiaoming90

High

An excessive number of user positions can be settled even if the shortfall allocated balance needed for fulfilling the close request is only a small amount

Summary

An excessive number of user positions can be settled even if the shortfall allocated balance needed for fulfilling the close request is only a small amount. As a result, victim's losses will be realized prematurely at unfavorable market conditions and prices, without giving them the opportunity to react or potentially recover from the temporary downturn. As a result, the victim is unfairly exposed to significant financial harm, having their positions closed at the worst possible moments, leading to a loss of assets for them. In this scenario, the victims are Symm Hedger (Party B1), Capital Hedger (Party B2), and Rasa Hedger (Party B3) in the report.

Root Cause

• An excessive number of user positions can be settled even if the shortfall allocated balance needed for fulfilling the close request is only a small amount

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Assuming a similar scenario mentioned in the Symm IO v0.8.3 documentation.

Bob (Party A) has five (5) open positions and zero allocated balance:

1. Position 1 with Symm Hedger (Party B1): Unrealized Profit: $300
2. Position 2 with Capital Hedger (Party B2): Unrealized Profit: $300
3. Position 3 with Rasa Hedger (Party B3): Unrealized Profit: $200
4. Position 4 with PerpsHub Hedger (Party B4): Unrealized Profit: $200
5. Position 5 with PerpsHub Hedger (Party B4): Unrealized Loss: $250

Bob wants to close Position 5, which has an unrealized loss of \$250. However, since Bob has zero allocated balance, he cannot cover this loss upon closing.

PerpsHub Hedger (Party B4) cannot fill Bob's close request for Position 5 because Bob doesn't have enough allocated balance to cover the \$250 loss. Even though Bob has an overall positive UPNL (\$750 net profit), his profits are unrealized and cannot be used to settle the loss directly.

Thus, the solution provided by the newly implemented settleUpnl feature is to realized the \$200 profit for Position 4 and \$50 profit for Position 3. Settling these PnLs will increase Bob's allocated balance by \$250. PerpsHub can now successfully fill the close request for Position 5, as Bob has sufficient allocated balance (\$250) to cover the loss (\$250).

However, the issue is that within the settleUpnl function, there is no limit on how many positions the hedger/PartyB (PerpsHub) can settle the user's positions. It can effectively settle all of the user's positions (Position 1-5) even if the shortfall allocated balance needed for fulfilling the close request is only a small amount (\$250 shortfall), resulting in an excessive amount of PnL being settled for no good reason. In this case, the hedger can go ahead and settle the entire $1000 profit instead of only the \$250 that is required.

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

File: LibSettlement.sol
15:     function settleUpnl(
16:         SettlementSig memory settleSig,
17:         uint256[] memory updatedPrices,
18:         address partyA,
19:         bool isForceClose
20:     ) internal returns (uint256[] memory newPartyBsAllocatedBalances) {
..SNIP..
40:         for (uint8 i = 0; i < settleSig.quotesSettlementsData.length; i++) {
41:             QuoteSettlementData memory data = settleSig.quotesSettlementsData[i];
42:             Quote storage quote = quoteLayout.quotes[data.quoteId];
43:             require(quote.partyA == partyA, "LibSettlement: PartyA is invalid");
..SNIP..
68:             if (quote.positionType == PositionType.LONG) {
69:                 settleAmounts[data.partyBUpnlIndex] +=
70:                     ((int256(updatedPrices[i]) - int256(quote.openedPrice)) * int256(LibQuote.quoteOpenAmount(quote))) /
71:                     1e18;
72:             } else {
73:                 settleAmounts[data.partyBUpnlIndex] +=
74:                     ((int256(quote.openedPrice) - int256(updatedPrices[i])) * int256(LibQuote.quoteOpenAmount(quote))) /
75:                     1e18;
76:             }
77:             quote.openedPrice = updatedPrices[i];
78:         }

Impact

An excessive number of user positions can be settled even if the shortfall allocated balance needed for fulfilling the close request is only a small amount. As a result, victim's losses will be realized prematurely at unfavorable market conditions and prices, without giving them the opportunity to react or potentially recover from the temporary downturn. As a result, the victim is unfairly exposed to significant financial harm, having their positions closed at the worst possible moments, leading to a loss of assets for them. In this scenario, the victims are Symm Hedger (Party B1), Capital Hedger (Party B2), and Rasa Hedger (Party B3) in the report.

PoC

No response

Mitigation

Consider limiting the maximum amount of profits that the hedgers can settle to the shortfall. In this example, the shortfall is \$250. Thus, the hedgers should not be allowed to settle more than \$250 to prevent excessive settlement of PartyA's PnL.

This issue also applied to the other side, where Party A needed to settle Party B's PnL during force close. Thus, the relevant mitigation also needs to be applied for this.

[MoonKnightDev]

There is no issue with settling more positions than needed. The settling of a position means that the unrealized P&L (uPnL) becomes realized P&L (PnL) for the parties involved, and it does not affect the trading rules.