Skip to content

Latest commit

 

History

History
198 lines (167 loc) · 10.5 KB

2023-08-Dopex.md

File metadata and controls

198 lines (167 loc) · 10.5 KB
Raw

Dopex

Contest Summary

Code under review: 2023-08-Dopex (2264 nSLOC)

Contest Page: Dopex-contest

Findings

[H-1]Incorrect precision assumed from RdpxPriceOracle creates multiple issues related to value inflation/deflation

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L372 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L381 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L539 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1160

Vulnerability details

Impact

The RdpxEthPriceOracle, available in the contest repo here, provides the RdpxV2Core, the UniV2LiquidityAmo and the PerpetualAtlanticVault contracts the necessary values for rdpx related price calculations.

The issue is that these contracts expect the returned values to be in 1e8 precision (as stated in the natspec here, here and here). But the returned precision is actually 1e18.

This difference creates multiple issues throughout the below contracts:

Contract Function Effect
rdpxV2Core.sol getRdpxPrice() Returns a 1e18 value when 1e8 expected
calculateBondCost() Deflates the rdpxRequired
calculateAmounts() Inflates the rdpxRequiredInWeth
_transfer() Inflates rdpxAmountInWeth and may cause possible underflow
UniV2LiquidityAmo.sol getLpPriceInEth() Overestimates the lp value
ReLp.sol reLP() Inflates min token amounts
PerpetualAtlanticVault.sol getUnderlyingPrice() Returns 1e18 instead of 1e8
calculatePremium() Inflates the premium calculation

Proof of Concept

In RdpxEthPriceOracle.sol file it exposes the following functions used in the contests:

getLpPriceInEth()
getRdpxPriceInEth()

These two functions provide the current price denominated in ETH, with a precision in 1e18, as confirmed by their respective natspec comments:

    /// @dev Returns the price of LP in ETH in 1e18 decimals
    function getLpPriceInEth() external view override returns (uint) {
    ...

    /// @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) {

But, in the contracts from the contest repo, the business logic (and even the natspec) assumes the returned precision will be 1e8. See below:

In the RdpxV2Core contract the assumption that the price returned from the oracle is clearly noted in the natspec of the getRdpxPrice() function:

  /**
   * @notice Returns the price of rDPX against ETH
   * @dev    Price is in 1e8 Precision
   * @return rdpxPriceInEth rDPX price in ETH
   **/
  function getRdpxPrice() public view returns (uint256) {
    return
      IRdpxEthOracle(pricingOracleAddresses.rdpxPriceOracle)
        .getRdpxPriceInEth();
  }

In UniV2LiquidityAmo the assumption is noted here:

  /**
   * @notice Returns the price of a rDPX/ETH Lp token against the alpha token
   * @dev    Price is in 1e8 Precision
   * @return uint256 LP price
   **/
  function getLpPrice() public view returns (uint256) {
And it has business logic implication here:

  function getLpTokenBalanceInWeth() external view returns (uint256) {
    return (lpTokenBalance * getLpPrice()) / 1e8;
  }

In PerpetualAtlanticVault it is noted here:

  /**
   * @notice Returns the price of the underlying in ETH in 1e8 precision
   * @return uint256 the current underlying price
   **/
  function getUnderlyingPrice() external view returns (uint256);

And the business logic implications in this contract are primarily found in the calculatePremium function, where the premium is divided by 1e8:

  function calculatePremium(
    uint256 _strike,
    uint256 _amount,
    uint256 timeToExpiry,
    uint256 _price
  ) public view returns (uint256 premium) {
    premium = ((IOptionPricing(addresses.optionPricing).getOptionPrice(
      _strike,
      _price > 0 ? _price : getUnderlyingPrice(),
      getVolatility(_strike),
      timeToExpiry
    ) * _amount) / 1e8);
  }

From the contest files it's clear that the assumption was that the returned price would be in 1e8, but this is Dopex's own RdpxPriceOracle, so was likely a simple oversight which slipped through testing as a MockRdpxEthPriceOracle was implemented to simplify testing, which mocked the values from the oracle, but only to a 1e8 precision.

Tools Used

Manual review. Communicated with sponsor to confirm RdpxEthPriceOracle.

Recommended Mitigation Steps

For price feeds where WETH will be token B, it is convention (although not a standard, as far as the reviewer is aware), that the precision returned will be 1e18. See here.

As the sponsor indicated that the team might move to Chainlink oracles, it is suggested to modify the RdpxV2Core, PerpetualAtlanticVault, UniV2Liquidity and the ReLp contracts to work with the returned 1e18 precision, assuming that the keep the token pair as rdpx/WETH.

Assessed type

Oracle

[H-2] Improper precision of strike price calculation can result in broken protocol

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1189-L1190 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L576-L583

Vulnerability details

Impact

Due to a lack of adequate precision, the calculated strike price for a PUT option for rDPX is not guaranteed to be 25% OTM, which breaks core assumptions around (1) protecting downside price movement of the rDPX which makes up part of the collateral for dpxETH & (2) not overpaying for PUT option protection.

More specifically, the price of rDPX as used in the calculateBondCost function of the RdpxV2Core contract is represented as ETH / rDPX, and is given in 8 decimals of precision. To calculate the strike price which is 25% OTM based on the current price, the logic calls the roundUp function on what is effectively 75% of the current spot rDPX price. The issue is with the roundUp function of the PerpetualAtlanticVault contract, which effectively imposes a minimum value of 1e6.

Considering approximate recent market prices of $2000/ETH and $20/rDPX, the current price of rDPX in 8 decimals of precision would be exactly 1e6. Then to calculate the 25% OTM strike price, we would arrive at a strike price of 1e6 * 0.75 = 75e4. The roundUp function will then round up this value to 1e6 as the strike price, and issue the PUT option using that invalid strike price. Obviously this strike price is not 25% OTM, and since its an ITM option, the premium imposed will be significantly higher. Additionally this does not match the implementation as outlined in the docs.

Proof of Concept

When a user calls the bond function of the RdpxV2Core contract, it will calculate the rdpxRequired and wethRequired required by the user in order to mint a specific _amount of dpxETH, which is calculated using the calculateBondCost function:

function bond(
  uint256 _amount,
  uint256 rdpxBondId,
  address _to
) public returns (uint256 receiptTokenAmount) {
  _whenNotPaused();
  // Validate amount
  _validate(_amount > 0, 4);

  // Compute the bond cost
  (uint256 rdpxRequired, uint256 wethRequired) = calculateBondCost(
    _amount,
    rdpxBondId
  );
  ...
}

Along with the collateral requirements, the wethRequired will also include the ETH premium required to mint the PUT option. The amount of premium is calculated based on a strike price which represents 75% of the current price of rDPX (25% OTM PUT option). In the calculateBondCost function:

function calculateBondCost(
  uint256 _amount,
  uint256 _rdpxBondId
) public view returns (uint256 rdpxRequired, uint256 wethRequired) {
  uint256 rdpxPrice = getRdpxPrice();

  ...

  uint256 strike = IPerpetualAtlanticVault(addresses.perpetualAtlanticVault)
    .roundUp(rdpxPrice - (rdpxPrice / 4)); // 25% below the current price

  uint256 timeToExpiry = IPerpetualAtlanticVault(
    addresses.perpetualAtlanticVault
  ).nextFundingPaymentTimestamp() - block.timestamp;
  if (putOptionsRequired) {
    wethRequired += IPerpetualAtlanticVault(addresses.perpetualAtlanticVault)
      .calculatePremium(strike, rdpxRequired, timeToExpiry, 0);
  }
}

As shown, the strike price is calculated as:

uint256 strike = IPerpetualAtlanticVault(addresses.perpetualAtlanticVault).roundUp(rdpxPrice - (rdpxPrice / 4)); It uses the roundUp function of the PerpetualAtlanticVault contract which is defined as follows:

function roundUp(uint256 _strike) public view returns (uint256 strike) {
  uint256 remainder = _strike % roundingPrecision;
  if (remainder == 0) {
    return _strike;
  } else {
    return _strike - remainder + roundingPrecision;
  }
}

In this contract roundingPrecision is set to 1e6, and this is where the problem arises. As I mentioned earlier, take the following approximate market prices: $2000/ETH and $20/rDPX. This means the rdpxPrice, which is represented as ETH/rDPX in 8 decimals of precision, will be 1e6. To calculate the strike price, we get the following: 1e6 * 0.75 = 75e4. However this value is fed into the roundUp function which will convert the 75e4 to 1e6. This value of 1e6 is then used to calculate the premium, which is completely wrong. Not only is 1e6 not 25% OTM, but it is actually ITM, meaning the premium will be significantly higher than was intended by the protocol design.

Tools Used

Manual review

Recommended Mitigation Steps

The value of the roundingPrecision is too high considering reasonable market prices of ETH and rDPX. Consider decreasing it.

Assessed type

Math