Dopex - circlelooper's results

Lines of code

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

Vulnerability details

Impact

roundingPrecision is too large for PUT option, strike can be rounded up to a price much higher than 25% below the current price.

Proof of Concept

The PUT option's strike price is expected to be 25% below the current price.

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

Let's examine the roundUp function.

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

If strike price (_strike) is less than roundingPrecision, the remainder is _strike and the price would be (_strike - _strike + roundingPrecision) = roundingPrecision. roundingPrecision is set to 1e6, so the PUT option's strike price would no smaller than 1e6. Please note the price is rdpx/ETH price and is in 8 decimals, if rdpx's current price is less than 1e6/1e8, the PUT option's strike price would still be 1e6, results in strike price is much higher than the 25% below the current price.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this issue, please set roundingPrecision to a much smaller value.

Assessed type

Decimal

Lines of code

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

Vulnerability details

Impact

Should update PerpetualAtlanticVaultLP collateral balance when settling an option or settle operation can be DOSed.

Proof of Concept

subtractLoss is called in settle function to ensure enough collateral was sent out.

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

The require statement in subtractLoss function requires that the collateral balance of the PerpetualAtlanticVaultLP should be strictly equal to (_totalCollateral - loss).

      collateral.balanceOf(address(this)) == _totalCollateral - loss

Please note _totalCollateral is not updated when settling the option, an attacker can front-run and send 1 wei amount of collateral token to PerpetualAtlanticVaultLP, then the require statement will fail due to inequality.

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this issue, please update PerpetualAtlanticVaultLP collateral balance when settling an option.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Price data returned by IRdpxEthOracle is in 18 decimals and is not converted to be in 8 decimals before using.

Proof of Concept

In PerpetualAtlanticVault.sol, getUnderlyingPrice function calls IRdpxEthOracle.getRdpxPriceInEth() to get rdpx price.

  function getUnderlyingPrice() public view returns (uint256) {
    return IRdpxEthOracle(addresses.assetPriceOracle).getRdpxPriceInEth();
  }

And the rdpx price is used to check if vault has enough collateral to write the options.

    uint256 currentPrice = getUnderlyingPrice(); // price of underlying wrt collateralToken
    uint256 strike = roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price
    IPerpetualAtlanticVaultLP perpetualAtlanticVaultLp = IPerpetualAtlanticVaultLP(
        addresses.perpetualAtlanticVaultLP
      );


    // Check if vault has enough collateral to write the options
    uint256 requiredCollateral = (amount * strike) / 1e8;

Here the price is treated to be in 8 decimals so (amount * strike) is divided by 1e8. In fact, the price is in 18 decimals, as we can find it here.

    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

To mitigate this issue, please converted the price to be in 8 decimals before using.

Assessed type

Decimal