code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

getLpPriceInEth and getRdpxPriceInEth return prices in 1e18 decimals, but we use it as 1e8 decimals #2131

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1206 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1238 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L373 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L605 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L669 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L673 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L276 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L335 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L550

Vulnerability details

Impact

Wrong decimals/price if we use RdpxEthOracle.sol as the oracle.

Proof of Concept

rdpx/eth oracle is not in the scope of this audit, so we can assure they are correct and only check if we use the API right. According to lib/rdpx-eth-oracle/src/RdpxEthOracle.sol, both the getLpPriceInRdpx() and getRdpxPriceInEth() return price in 1e18 decimals:

    /// @dev Returns the price of LP in ETH in 1e18 decimals
    function getLpPriceInEth() external view override returns (uint) {
        // .....
        return (lpPriceIn112x112 * 1e18) >> 112;
    }

    /// @notice Returns the price of rDPX in ETH
    /// @return price price of rDPX in ETH in 1e18 decimals
    function getRdpxPriceInEth() external view override returns (uint price) {
        require(
            blockTimestampLast + timePeriod + nonUpdateTolerance >
                block.timestamp,
            "RdpxEthOracle: UPDATE_TOLERANCE_EXCEEDED"
        );

        price = consult(token0, 1e18);

        require(price > 0, "RdpxEthOracle: PRICE_ZERO");
    }

However, we assumed the decimals is 1e18 in the current implementation:

  /**
   * @notice Returns the price of a rDPX/ETH Lp token against the ETH token
   * @dev    Price is in 1e8 Precision @audit-issue 1e18
   * @return uint256 LP price
   **/
  function getLpPrice() public view returns (uint256) {
    return
      IRdpxEthOracle(pricingOracleAddresses.rdpxPriceOracle).getLpPriceInEth();
  }

  function getRdpxPrice() public view returns (uint256) {
    return
      IRdpxEthOracle(pricingOracleAddresses.rdpxPriceOracle)
        .getRdpxPriceInEth();
  }

Thie can lead to wrong decimals/price if we use RdpxEthOracle.sol as the oracle, for example in _calculateAmounts():

  function _calculateAmounts(
    uint256 _wethRequired,
    uint256 _rdpxRequired,
    uint256 _amount,
    uint256 _delegateFee
  ) internal view returns (uint256 amount1, uint256 amount2) {
    // Commented below for better clarity
    uint256 rdpxRequiredInWeth = (_rdpxRequired * getRdpxPrice()) / 1e8; // @audit-issue 1e18

Tools Used

Manual Review.

Recommended Mitigation Steps

Change 1e8 to 1e18.

Assessed type

Math

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #1075

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #549

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report

c4-judge commented 11 months ago

GalloDaSballo marked the issue as satisfactory

c4-judge commented 11 months ago

GalloDaSballo changed the severity to 3 (High Risk)