sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

J4de - `BaseOracleExt.sol#_isValidPrices` should be divided by `minPrice` instead of `maxPrice` #67

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

J4de

medium

BaseOracleExt.sol#_isValidPrices should be divided by minPrice instead of maxPrice

Summary

BaseOracleExt.sol#_isValidPrices should be divided by minPrice instead of maxPrice

Vulnerability Detail

 File: oracle/BaseOracleExt.sol
 22     function _isValidPrices(
 23         uint256 price0,
 24         uint256 price1,
 25         uint256 maxPriceDeviation
 26     ) internal pure returns (bool) {
 27         uint256 maxPrice = price0 > price1 ? price0 : price1;
 28         uint256 minPrice = price0 > price1 ? price1 : price0;
 29         return
 30             (((maxPrice - minPrice) * Constants.DENOMINATOR) / maxPrice) <=
 31             maxPriceDeviation;
 32     }

_isValidPrices function is used to calculate whether the difference between two prices exceeds a certain percentage, so as to determine whether the price is available. Generally speaking percentages should be divided by small numbers rather than large numbers. There will be a large numerical difference between the two.

For example, price0 is 100 and price1 is 120,

  1. Normally their difference percentage is (120 - 100) / 100 = 20.00%
  2. Calculated by _isValidPrices is (120 - 100) / 120 = 16.67%

Impact

This will lead to a wider range of price deviations than expected, which may ultimately lead to more volatile prices.

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/oracle/BaseOracleExt.sol#L29-L30

Tool used

Manual Review

Recommendation

        return
-           (((maxPrice - minPrice) * Constants.DENOMINATOR) / maxPrice) <=
+           (((maxPrice - minPrice) * Constants.DENOMINATOR) / minPrice) <=
            maxPriceDeviation;