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

0 stars 0 forks source link

safdie - Precision issues for `pnl` and `fee` will cause incorrect financial decisions based on the wrong profit/loss values in `LibQuote.sol` #29

Open sherlock-admin3 opened 1 week ago

sherlock-admin3 commented 1 week ago

safdie

Medium

Precision issues for pnl and fee will cause incorrect financial decisions based on the wrong profit/loss values in LibQuote.sol

Summary

Precision issues for pnl and fee in LibQuote.sol will cause incorrect financial decisions based on the wrong profit/loss values.

Root Cause

Solidity does not support floating-point numbers. Instead, it uses fixed-point arithmetic, which can lead to precision loss when dealing with very large or very small numbers.

Operations involving division and multiplication by large constants (like 1e18 or 1e36) can introduce rounding errors. These errors occur because Solidity truncates the decimal part of the result, leading to a loss of precision.

The uint256 data type, while capable of handling large integers, does not inherently support decimal places. This necessitates manual scaling (e.g., multiplying by 1e18), which can introduce precision issues if not handled carefully.

The calculations for pnl and fee involve division by 1e18 and 1e36 respectively. https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibQuote.sol#L139 https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibQuote.sol#L159 https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibQuote.sol#L161

Consider the calculation of pnl in getValueOfQuoteForPartyA function:

pnl = ((currentPrice - quote.openedPrice) * filledAmount) / 1e18;

If currentPrice and quote.openedPrice are very close, the subtraction might result in a very small number, which, when multiplied and then divided by 1e18, can lead to a loss of precision.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

  1. The pnl calculation results in -1, which is incorrect and misleading. This could lead to incorrect financial decisions based on the wrong profit/loss values.
  2. Over multiple transactions, these small precision errors can accumulate, leading to significant financial discrepancies and potential losses.
  3. Users may lose trust in the system if they notice discrepancies in their profit/loss calculations, which could affect the platform’s reputation.

PoC

Suppose currentPrice is $1,000.123456789012345678 and quote.openedPrice is $1,000.123456789012345679. The difference is extremely small, but due to precision limitations, it could be rounded off incorrectly.

uint256 currentPrice = 1000123456789012345678; // Representing $1,000.123456789012345678 in wei
uint256 openedPrice = 1000123456789012345679; // Representing $1,000.123456789012345679 in wei
uint256 filledAmount = 1000000000000000000; // 1 unit in wei

uint256 pnl = ((currentPrice - openedPrice) * filledAmount) / 1e18;
// pnl = ((1000123456789012345678 - 1000123456789012345679) * 1000000000000000000) / 1e18;
// pnl = (-1 * 1000000000000000000) / 1e18;
// pnl = -1

Mitigation

  1. Utilize libraries like ABDKMathQuad or FixedPoint to handle high-precision arithmetic operations.
  2. Instead of using 1e18 and 1e36, consider using higher precision multipliers if the underlying data supports it.