Dopex - Juntao's results

Lines of code

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

Vulnerability details

Impact

PUTS Options may be created with strike price being higher than the current price, this leads to more funds required by vault and may cause insolvency.

Proof of Concept

Users call bond(...) or bondWithDelegate(...) to do bonding, both of the functions will call _purchaseOptions(...) to purchase rdpx put options from PerpetualAtlanticVault if putOptionsRequired is true. In PerpetualAtlanticVault, purchase(...) will then be called to create the put option, the strike price is determined as below:

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

Let's assume rdpx price is 10u and ETH price is 2000u, then currentPrice is 0.005e8, strike is calculated and rounded up by roundUp(0.005e8 - (0.005e8 / 4)):

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

The strike price is 0.01e8, higher than currentPrice.

The root cause is price is in 1e8 decimals and roundingPrecision is defined as 1e6, which means any price less than 1% of the ETH price will be rounded up to 1% of the ETH price, which is 20u in the above case.

Tools Used

Manual Review

Recommended Mitigation Steps

roundingPrecision should be defined as 1e4 or even smaller, 0.01% of the ETH price.

Assessed type

Decimal

Lines of code

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

Vulnerability details

Impact

Malicious user can grief attack to prevent from option being settled.

Proof of Concept

When an option is settled, protocol will transfer collateral from PerpetualAtlanticVaultLP:

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

and subtract loss from total available assets from PerpetualAtlanticVaultLP:

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

The [subtractLoss(...)] will check if the transferred collateral balance is equal to _totalCollateral - loss:

    require(
      collateral.balanceOf(address(this)) == _totalCollateral - loss,
      "Not enough collateral was sent out"
    );

A malicious user can simply transfer 1 wei collateral token to PerpetualAtlanticVaultLP, then the settle transaction will be reverted due to mismatch.

Please copy below test codes to test/rdpxV2-core/Unit.t.sol to verify:

  function testAuditGriefAttackSettle() public {
    rdpxV2Core.bond(1 ether, 0, address(this));
    vault.addToContractWhitelist(address(rdpxV2Core));
    
    uint256[] memory _ids = new uint256[](1);
    _ids[0] = 0;
    rdpxPriceOracle.updateRdpxPrice(1e7);

    // attacker sends 1 wei to vaultLP
    address attacker = address(uint160(uint(keccak256(abi.encodePacked("attacker")))));
    deal(address(vaultLp.collateral()), attacker, 1 wei);
    vm.prank(attacker);
    vaultLp.collateral().transfer(address(vaultLp), 1 wei);

    vm.expectRevert("Not enough collateral was sent out");
    rdpxV2Core.settle(_ids);
  } 

Tools Used

Manual Review

Recommended Mitigation Steps

Add sync() function to PerpetualAtlanticVaultLP:

  function sync() public {
    _totalCollateral = collateral.balanceOf(address(this));
  }

Call sync() function before transferring collateral token from PerpetualAtlanticVaultLP.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1238-L1242 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L529-L531 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L221-L222

Vulnerability details

Impact

Protocol expects oracle returns price in 8 decimals, while the oracle returns price in 18 decimals, this mismatch will cause huge loss to the protocol.

Proof of Concept

In RdpxV2Core contract, getRdpxPrice() function calls IRdpxEthOracle.getRdpxPriceInEth() to get the price of rDPX against ETH:

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

In PerpetualAtlanticVault contract, getUnderlyingPrice() calls function calls IRdpxEthOracle.getRdpxPriceInEth() to get the price of rDPX against ETH:

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

In ReLPContract contract, IRdpxEthOracle.getRdpxPriceInEth() is also called in reLP(...) to get rdpx price:

    tokenAInfo.tokenAPrice = IRdpxEthOracle(addresses.rdpxOracle)
      .getRdpxPriceInEth();

The protocol expects the price to be in 8 decimals, however, the implementation of IRdpxEthOracle.getRdpxPriceInEth() in fact returns in 18 decimals:

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

Price data returned by IRdpxEthOracle.getRdpxPriceInEth() should be divided by 1e10 to convert the data to be in 1e8 decimals.

Assessed type

Oracle