Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

panprog - PythOracle _recordPrice() function incorrectly treats expo (exponent) from PythStructs.Price which can lead to protocol malfunction and loss of funds after oracle switch #45

Closed
sherlock-admin opened this issue Aug 15, 2023 · 1 comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Aug 15, 2023

panprog

medium

PythOracle _recordPrice() function incorrectly treats expo (exponent) from PythStructs.Price which can lead to protocol malfunction and loss of funds after oracle switch

Summary

When pyth price is recorded in PythOracle, it converts price provided by pyth to Fixed6 format. However, it treats expo incorrectly: multiplying price by 10 power absolute value of expo:

        _prices[oracleVersion] = Fixed6Lib.from(price.price)
            .mul(Fixed6Lib.from(SafeCast.toInt256(10 ** SafeCast.toUint256(price.expo > 0 ? price.expo : -price.expo))));

For negative expo, price will be incorrect. Moreover, if oracle price is changed to a different number of digits, resulting final price will unexpectedly increase or decrease orders of magnitude from pre-switch price, creating huge unexpected funds loss for many users of the protocol.

Vulnerability Detail

Pyth stores its price in a structure with price itself (int64) and price exponent (int32). Exponent can be both positive and negative. For example, price 123.4567 will be stored as:

  • price = 1234567
  • expo = -4
    The converted price should be calculated as 1234567 * 10^-4 = 123.4567
    Large prices can be stored with positive exponent, for example price 12345670000 can be stored as
  • price = 1234567
  • expo = 4

However, in both cases PythOracle will convert the price to Fixed6(1234567) * Fixed6(10^4) = Fixed6(12345670000)

This can lead to a serious problem if, for example, pyth price feed is switched for the same market from 4 decimals precision to 5:

  1. Pyth feed 1: 123.4567 price is represented as (price: 1234567, expo: -4), converted by PythOracle to Fixed6(12345670000)
  2. Pyth feed 2: 123.4567 price is represented as (price: 12345670, expo: -5), converted by PythOracle to Fixed6(123456700000) (10 times the price of feed1)

Since most Pyth price feeds use negative expo, this is quite feasible scenario: if some token price falls significantly, it might need better precision and thus the oracle price feed will be switched to a price with a higher precision, which will make the price unexpectedly grow by orders of magnitude, leading to a chaos and loss of funds for many protocol users due to sudden price jump.

Impact

Incorrect oracle price for negative expo pyth price feeds which can lead to chaos and loss of funds if price feed is switched to a different-precision feed for the same price.

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/pyth/PythOracle.sol#L202-L203

Tool used

Manual Review

Recommendation

Check the sign of expo in pyth price and divide by 10**-expo if it's negative. However, since price is Fixed6 (just 6 decimals), this might lose precision for very small prices (if expo is less than -6), maybe consider switching to Fixed18 or handle it in some other way.

Duplicate of #56

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Aug 18, 2023
@sherlock-admin
Copy link
Contributor Author

1 comment(s) were left on this issue during the judging contest.

141345 commented:

d

@sherlock-admin sherlock-admin changed the title Wonderful Silver Loris - PythOracle _recordPrice() function incorrectly treats expo (exponent) from PythStructs.Price which can lead to protocol malfunction and loss of funds after oracle switch panprog - PythOracle _recordPrice() function incorrectly treats expo (exponent) from PythStructs.Price which can lead to protocol malfunction and loss of funds after oracle switch Aug 23, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant