xiaoming90 - Ineffective settlement cooldown measure

[sherlock-admin3]

xiaoming90

High

Ineffective settlement cooldown measure

Summary

An ineffective settlement cooldown measure allows hedgers to settle any number of quotes/positions involving other hedgers in one go (e.g., within a single block/TX). As a result, it could lead to asset loss for PartyB (victim) because another trader (the exploiter) can settle positions prematurely at times that are disadvantageous to PartyB (victim)

For example, in a highly volatile market, if PartyB’s positions temporarily incur a loss due to sudden market fluctuations, the exploiter could immediately settle these positions before the market has a chance to recover. This premature settlement forces Party B to realize losses that might have otherwise been avoided if the positions had remained open.

Root Cause

1. The cooldown mapping key is set based on [msg.sender][partyB][partyA] instead of [msg.sender][partyB]
2. Hedgers can settle as any number of quotes/positions involving other hedgers in one go (e.g., within a single block/TX) as long as the PartyA differs each time.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

Assume two PartyB/hedgers

• PartyB_1 = Rasa
• PartyB_2 = PerpsHub

PartyB_1 (Rasa) has five (5) quotes with different PartyAs.

• Quote 1 - PartyB_1 with PartyA_1
• Quote 2 - PartyB_1 with PartyA_2
• Quote 3 - PartyB_1 with PartyA_3
• Quote 4 - PartyB_1 with PartyA_4
• Quote 5 - PartyB_1 with PartyA_5

According to the documentation, the cooldown mechanism prevents a hedger from excessively closing the positions of other hedgers.

Cooldown Mechanism: To prevent hedgers from repeatedly settling UPNL for quotes involving other hedgers, a cooldown period is enforced. A hedger cannot settle UPNL for another hedger's quotes more than once in a period.

However, PartyB_2 (PerpsHub) can close all five (5) quotes of PartyB_1 (Rasa) without being subjected to any cooldown or restriction (as long as the PartyA is different), which is excessive.

1. PartyB_2 close Quote 1 of PartyB_1. lastUpnlSettlementTimestamp[PartyB_2][PartyB_1][PartyA_1] is zero at this point. Thus, the cooldown check passes. lastUpnlSettlementTimestamp[PartyB_2][PartyB_1][PartyA_1] will be set to the current time (block.timestamp) at the end.
2. PartyB_2 close Quote 2 of PartyB_1. lastUpnlSettlementTimestamp[PartyB_2][PartyB_1][PartyA_2] is zero at this point. Thus, the cooldown check passes. lastUpnlSettlementTimestamp[PartyB_2][PartyB_1][PartyA_2] will be set to the current time (block.timestamp) at the end.

3. PartyB_2 repeats the same for Quote 3, 4, and 5 of PartyB_1.

   If PartyB_1 has 50 quotes/positions, PartyB_2 can proceed to close all of them in one go.

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

File: LibSettlement.sol
89:             if (!isForceClose && msg.sender != partyB) {
90:                 require(
91:                     block.timestamp >=
92:                         MAStorage.layout().lastUpnlSettlementTimestamp[msg.sender][partyB][partyA] + MAStorage.layout().settlementCooldown,
93:                     "LibSettlement: Cooldown should be passed"
94:                 );
95:                 MAStorage.layout().lastUpnlSettlementTimestamp[msg.sender][partyB][partyA] = block.timestamp;
96:             }

Impact

The issue could lead to asset loss for PartyB (victim) because another trader (the exploiter) can settle positions prematurely at times that are disadvantageous to PartyB.

For example, in a highly volatile market, if PartyB’s positions temporarily incur a loss due to sudden market fluctuations, the exploiter could immediately settle these positions before the market has a chance to recover. This premature settlement forces PartyB to realize losses that might have otherwise been avoided if the positions had remained open.

PoC

No response

Mitigation

The number of quotes/positions involving other hedgers that a hedger can settle in one go (e.g., within a single block/TX) must be restricted (e.g., maximum three positions within a single block/TX + cooldown period). The codebase can track the number of quotes/positions involving other hedgers that a hedger has already settled within the current block/TX and revert if it exceeds the maximum limit.

Alternatively, a more restrictive approach can be adopted. The mapping index for the cooldown should be per caller:partyB instead of caller:partyB:partyA to prevent PartyB from excessively settling another hedger's quotes within the cooldown period.

if (!isForceClose && msg.sender != partyB) {
  require(
    block.timestamp >=
-      MAStorage.layout().lastUpnlSettlementTimestamp[msg.sender][partyB][partyA] + 
+      MAStorage.layout().lastUpnlSettlementTimestamp[msg.sender][partyB] + 
      MAStorage.layout().settlementCooldown,
    "LibSettlement: Cooldown should be passed"
  );
-  MAStorage.layout().lastUpnlSettlementTimestamp[msg.sender][partyB][partyA] = block.timestamp;
+  MAStorage.layout().lastUpnlSettlementTimestamp[msg.sender][partyB] = block.timestamp;
}