Dopex - crunch's results

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L270

Vulnerability details

Impact

If rDPX price is much lower than ETH price, PUT Option strike price can be much higher than 25% below the current price, this violates the invariant that PUT Option can only be created with strike price is 25% below the current price.

Proof of Concept

As stated in the code comment, PUT Option strike price should be 25% below the current price:

    uint256 strike = roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price

The currentPrice is rDPX price in 8 dp denominated in ETH:

    uint256 currentPrice = getUnderlyingPrice(); // price of underlying wrt collateralToken

Currently, rDPX price is 20u and ETH price is 1700u, then the current price is 0.011e8 and PUT Option strike price should be 0.00825e8. However, the strike price is rounded up by roundUp method:

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

=> remainder = _strike % roundingPrecision = 0.00825e8 % 1e6 = 0.00825e8 != 0 => strike = _strike - remainder + roundingPrecision = 0.00825e8 - 0.00825e8 + 1e6 = 1e6 => PUT Option strike price is much higher than 25% below the current price

Tools Used

Manual Review

Recommended Mitigation Steps

It is recommended to set roundingPrecision to a much smaller value so that PUT Option strike price won't be rounded up to be much higher than 25% below the current price.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L201

Vulnerability details

Impact

It is possible to grief attack on settling an PUT Option.

Proof of Concept

If an PUT Option is In-The-Money, it will be settled and collateral token will be transferred from perpetual vault LP to rdpx rdpxV2Core:

    collateralToken.safeTransferFrom(
      addresses.perpetualAtlanticVaultLP,
      addresses.rdpxV2Core,
      ethAmount
    );

After that, it will update collateral token balance of perpetual vault LP:

    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).subtractLoss(
      ethAmount
    );

It is worth noting that subtractLoss will check if enough collateral was sent out and it requires the collateral token balance of perpetual vault LP is equal to the _totalCollateral - loss:

  function subtractLoss(uint256 loss) public onlyPerpVault {
    require(
      collateral.balanceOf(address(this)) == _totalCollateral - loss,
      "Not enough collateral was sent out"
    );
    _totalCollateral -= loss;
  }

It is possible that an attacker grief attack on settling by sending some amount of collateral token to perpetual vault LP, then the require statement above will fail and settling transaction will revert.

Tools Used

Manual Review

Recommended Mitigation Steps

The culprit is that _totalCollateral is not synced before settling, it is recommended to do so to ensure settling transaction cannot be grief attacked.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1238-L1242

Vulnerability details

Impact

getRdpxPrice in RdpxV2Core.sol assumes IRdpxEthOracle returns RdpxPriceInEth in 8 dp which is not the case.

Proof of Concept

getRdpxPrice is used to

1. Calculate the amounts received by delegate and delegatee after accounting for delegate fee:

    uint256 rdpxRequiredInWeth = (_rdpxRequired * getRdpxPrice()) / 1e8;

2. Transfers rdpx or burns the equivalent in rDPX Decaying bonds:

      uint256 rdpxAmountInWeth = (_rdpxAmount * getRdpxPrice()) / 1e8;
      uint256 discountReceivedInWeth = _bondAmount -
        _wethAmount -
        rdpxAmountInWeth;
      uint256 extraRdpxToWithdraw = (discountReceivedInWeth * 1e8) /
        getRdpxPrice();

3. Calculate the cost of bonding:

    uint256 rdpxPrice = getRdpxPrice();

In all of the above used case, the price is divided by 1e8 because getRdpxPrice incorrectly assumes the price is in 8 dp, while in fact the IRdpxEthOracle returns RdpxPriceInEth in 8 dp is in 18 dp:

    /// @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");
    }

Tools Used

Manual Review

Recommended Mitigation Steps

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

Assessed type

Decimal