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

0 stars 0 forks source link

xiaoming90 - Rounding error when computing `settleAmounts` #48

Open sherlock-admin4 opened 1 week ago

sherlock-admin4 commented 1 week ago

xiaoming90

High

Rounding error when computing settleAmounts

Summary

A rounding error when computing settleAmounts could lead to a small amount of PnL will be lost each time. Since this is a trading protocol, it is expected that there will be users who are active traders or entities that perform high-frequency or algorithmic trading, resulting in the loss accumulating to a significant amount.

Root Cause

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

The settleAmounts is computed based on the following formula:

$\frac{(updatedPrice - openedPrice) quantity}{1e18}$ OR $\frac{(openedPrice - updatedPrice) quantity}{1e18}$

The issue is that in an edge case where the difference between updatedPrice and openedPrice is small, coupled with small quantity (LibQuote.quoteOpenAmount(quote)), the numerator of the equation will be larger than the denominator, leading to settleAmounts being rounded down to zero.

In this case, no PnL is settled to PartyA or PartyB's allocated balance (because settleAmounts is zero) , yet the quote's openedPrice is being updated to the updatedPrice at Line 77 below. When openedPrice is set to updatedPrice, this effectively means that the PnL has settled successfully and users have received the settled PnL to which they are entitled. However, that is not the case here due to rounding error.

A small amount of PnL will be lost each time the rounding error occurs. Since this is a trading protocol, and it is expected that there will be users who are active traders or entities that perform high-frequency or algorithmic trading, resulting in the loss accumulating to a significant amount.

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

File: LibSettlement.sol
15:     function settleUpnl(
..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

Lost of assets. A small amount of PnL will be lost each time the rounding error occurs. Since this is a trading protocol, it is expected that there will be users who are active traders or entities that perform high-frequency or algorithmic trading, resulting in the loss accumulating to a significant amount.

PoC

No response

Mitigation

Consider reverting the function and skip updating the quote.openedPrice if the settleAmount ends up being zero. If settleAmount is zero, there is also no point proceeding with the rest of the settleUpnl execution since there is nothing to settle anyway (The allocated balance of either PartyA or PartyB will not increase at the end).