sherlock-audit / 2023-09-perennial-judging

0 stars 0 forks source link

0xVinylDavyl - From the arithmetic operations in the _recordPrice function, specifically the variable expo6Decimal. If the value of expo6Decimal becomes extremely large or small, it could result in an integer overflow or underflow when performing operations on it. #17

Closed sherlock-admin2 closed 11 months ago

sherlock-admin2 commented 11 months ago

0xVinylDavyl

medium

From the arithmetic operations in the _recordPrice function, specifically the variable expo6Decimal. If the value of expo6Decimal becomes extremely large or small, it could result in an integer overflow or underflow when performing operations on it.

Summary

The "Potential Integer Overflow in _recordPrice Function" is a vulnerability in the contract's _recordPrice function, which performs arithmetic operations on the expo6Decimal variable. If not handled properly, these operations could lead to integer overflow or underflow issues.

Vulnerability Detail

The vulnerability arises from the arithmetic operations in the _recordPrice function, specifically the variable expo6Decimal. If the value of expo6Decimal becomes extremely large or small, it could result in an integer overflow or underflow when performing operations on it.

Impact

Integer overflow or underflow can lead to unexpected and potentially harmful behavior in smart contracts, including incorrect calculations, unexpected state changes, and even contract halting. In the context of this contract, it could affect the recorded price data, potentially leading to incorrect oracle information.

Code Snippet

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

function _recordPrice(uint256 oracleVersion, PythStructs.Price memory price) private {
    int256 expo6Decimal = 6 + price.expo;
    _prices[oracleVersion] = (expo6Decimal < 0) ?
        Fixed6.wrap(price.price).div(Fixed6Lib.from(UFixed6Lib.from(10 ** uint256(-expo6Decimal)))) :
        Fixed6.wrap(price.price).mul(Fixed6Lib.from(UFixed6Lib.from(10 ** uint256(expo6Decimal))));
    publishTimes[oracleVersion] = price.publishTime;
}

Tool used

Manual Review

Recommendation

To mitigate the potential integer overflow or underflow vulnerability in the _recordPrice function, follow these recommended steps:

  1. Use SafeMath Library: Implement safe arithmetic operations by using a library like OpenZeppelin's SafeMath. SafeMath provides functions for addition, subtraction, multiplication, and division that prevent integer overflow or underflow.

    import "@openzeppelin/contracts/utils/math/SafeMath.sol";
    
    // Inside the contract
    using SafeMath for uint256;
  2. Update Arithmetic Operations: Modify the _recordPrice function to use SafeMath for arithmetic operations. Ensure that the arithmetic operations are safe and do not result in integer overflow or underflow.

    function _recordPrice(uint256 oracleVersion, PythStructs.Price memory price) private {
       int256 expo6Decimal = int256(6) + price.expo;
       uint256 exponent = uint256(-expo6Decimal);
       uint256 divisor = 10 ** exponent;
    
       if (expo6Decimal < 0) {
           _prices[oracleVersion] = Fixed6.wrap(price.price).div(Fixed6Lib.from(UFixed6Lib.from(divisor)));
       } else {
           _prices[oracleVersion] = Fixed6.wrap(price.price).mul(Fixed6Lib.from(UFixed6Lib.from(divisor)));
       }
    
       publishTimes[oracleVersion] = price.publishTime;
    }

By implementing these recommendations, you can ensure that the arithmetic operations in the _recordPrice function are safe and do not result in integer overflow or underflow, enhancing the security of the contract.

sherlock-admin commented 11 months ago

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

panprog commented:

invalid because the real prices used in production won't overflow, and price feed provider is trusted

n33k commented:

invalid, ai generated groundless report

darkart commented:

The same as 14

polarzero commented:

Invalid. The contract uses Solidity 0.8.19, which has built-in integer overflow/underflow checks.