Dopex - 0Kage's results

Lines of code

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

Vulnerability details

Impact

PerpetualVaultLP::deposit computes shares for the deposit using the previewDeposit function. previewDeposit calculates shares proportional to the assets deposited v/s the totalCollateral which is the sum of ETH and RDPX collateral.

Note that perpetualAtlanticVault.updateFunding() is called AFTER shares are computed. updateFunding has the effect of increasing totalCollateral as the vault gets the funding fees from the options it has written to RdpxV2Core. Since total collateral used for calculating shares does not include the funding fees, depositors end up with marginally additional shares at the expense of existing LP's. In effect, this is like stealing value from existing LP's and sharing it with new depositors.

Proof of Concept

1. Bob is an existing LP having all the supply (100 shares) against 1 ETH as total collateral
2. Funding accrued since last update is equal to 0.25 ETH
3. Pete deposits 0.25 ETH into the vault
4. As per current calculation, Pete gets 0.25*100/1.25 = 20 shares
5. Actual calculation should have been 0.25*100/ (1.25 + 0.25) = 16 shares (rounded)

Pete got 4 additional shares at the expense of Bob because Pete's share calculation does not include the collateral already accrued in the form of funding fees.

Tools Used

Manual

Recommended Mitigation Steps

Call perpetualAtlanticVault.updateFunding() before calculating shares via previewDeposit(assets)

Assessed type

ERC4626

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L333 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L800 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/perp-vault/PerpetualAtlanticVault.sol#L376

Vulnerability details

In the current implementation of PerpetualAtlanticVault::settle, the only way an optionId can be burnt is when the option is in-the-money. Note that at the time of bonding, a put option that is 25% out of the money is purchased by user. If the RDPX-ETH price is above this price, the option is rolled over to the next epoch and a new premium is again calculated inside the PerpetualAtlanticVault::calculateFunding and added to the totalFundingForEpoch.

Effectively, as long as the option is out of money, the funding cost continues to be deducted from core contract and passed to Atlantic vault LPs. There is no check if the underlying bond for which this put option is minted is redeemed or not.

Not completely sure if this is bug or feature of Perpetual Atlantic Puts but since the utility of these puts is to protect downside price movements of RDPX during bonding, such options should be retired once the underlying bond is redeemed by user. Paying out a premium on a perennial basis to Atlantic LP's does not make sense.

Impact

There are 2 implications:

1. Atlantic vault LPs collateral will be locked so long as option is out of money. This is because optionStrikes[strike] never reduces so long as the option continues to be OTM.
2. Core contract continues to pay out funding costs for OTM options that cannot be settled.

Proof of Concept

Tools Used

Manual

Recommended Mitigation Steps

Consider settling optionIds that no longer have an underlying bonding. Put options for such bonds should expire and free up locked collateral in Atlantic Vaults.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/reLP/ReLPContract.sol#L273

Vulnerability details

Impact

In line 273 of ReLPContract, minTokenBAmount is calculated as:

    mintokenAAmount =
      (((amountB / 2) * tokenAInfo.tokenAPrice) / 1e8) -
      (((amountB / 2) * tokenAInfo.tokenAPrice * slippageTolerance) / 1e16);

In the above, tokenAInfo.tokenAPrice is calculated in Line 221 as:

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

To convert tokenB to tokenA, the inverse of tokenAPrice needs to be used. Formula above would give a negligible value for mintokenAAmount causing a huge slippage loss on tokenA (rDPX).

Proof of Concept

In the above case, tokenA corresponds to RDPX and tokenB corresponds to WETH. If we assume RDPX as $20 and ETH as $2000, rdpx:ETH = 0.01, ie 1 RDPX = 0.01 ETH. If amountB/2 is 1 ETH, then minTokenAAmount gives a value of 0.01 instead of 100 (for simplicity, I assume slippageTolerance=0). This would mean that this swap effectively has no slippage protection and bots can frontrun this transaction to cause significant slippage on RDPX.

Tools Used

Manual

Recommended Mitigation Steps

Recommend the following changes:

   uint256 tokenBPrice = IRdpxEthOracle(addresses.rdpxOracle)
      .getEthPriceInRdpx();

    mintokenAAmount =
      (((amountB / 2) * tokenBPrice) / 1e8) -
      (((amountB / 2) * tokenBPrice * slippageTolerance) / 1e16);

Assessed type

Uniswap

Lines of code

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

Vulnerability details

Documentation states that each epoch is 1 week long. Current contract has a provision to update funding duration via function call PerpetualAtlanticVault::updateFundingDuration. However, even a small increase in funding duration can lead to excessively high premiums for put option buyers. This is due to a flaw in the way nextFundingPaymentTimestamp is calculated. The PerpetualAtlanticVault::updateFundingDuration implementation is as follows:

  function nextFundingPaymentTimestamp()
    public
    view
    returns (uint256 timestamp)
  {
    return genesis + (latestFundingPaymentPointer * fundingDuration);
  }

Based on the actual value of latestFundingPaymentPointer, the next funding payment timestamp could be way ahead into the future even if the funding duration is increased by a small amount. For eg., a funding duration increase from 7 days to 8 days after 100 epochs pushes the nextFundingPaymentTimestamp 100 days ahead.

Note that when a bonding process happens, a user is paying the premium in collateral token (ETH) in the PerpetualAtlanticVault::purchase function. Time to expiry is calculated in line 283 as the next funding payment date minus current timestamp. Immediately after a minor increase in funding duration, the next put option buyer will buy an option with time to expiry as 100 days. Everything else being equal, as option duration increases, so does its premium. A user is paying for a 100 day option while his bonding process is just a few days long. This makes no sense and causes a heavy loss to the user participating in bonding process.

Impact

Price increase is non-linear and hence a long dated option will have much higher price than a regular epoch option. Since user is funding the premium, this is a direct loss to user.

Proof of Concept

1. Current funding duration is 7 days
2. 100 epochs have passed (latestFundingDurationPointer = 100)
3. Admin changes funding duration to 8 days
4. Next funding payment date is pushed forward by 100 days because the duration increase is applied in every epoch from genesis
5. User bonding pays a premium on a 100 day option. For a given vol and strike, 100 day expiry vastly bloats price of option

Tools Used

Manual

Recommended Mitigation Steps

nextFundingPaymentTimestamp calculation needs to be changed. Keep a storage variable previousFundingPaymentTimestamp that gets updated every time an epoch gets completed or in the updateFunding. This simply adds the funding duration to the previousFundingPaymentTimestamp to get the nextFundingPaymentTimestamp

Current implementation risks big jumps and unexpected changes even with minor change in fundingDuration

Assessed type

Math

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L172

Vulnerability details

Impact

UniV2LiquidityAMO is used for market operations that maintain the DpxETH:ETH peg. Whenever liquidity is added, removed or DpxETH is swapped for ETH or vice versa, the residual tokens are sent back to the RpdxV2Core contract using the UniV2LiquidityAMO::_sendTokensToRdpxV2Core. However, this function fails to sync the accounting balances with token balances by triggering the RpdxV2Core::sync function. This effectively means the token balances from market operations are not correctly updated in the core contract.

Note that, the UniV3LiquidityAMO does the syncing after the transfers

Not updating token balances

1. gives a wrong representation of reservers when RpdxV2Core::getReserveTokenInfo is called
2. Incorrect tokenBalance can cause underflow in RpdxV2Core::_purchaseOptions, RpdxV2Core::stake or ``RpdxV2Core::settle` functions

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Manual

Recommended Mitigation Steps

Call IRdpxV2Core(rdpxV2Core).sync() after making all the transfers inside the UniV2LiquidityAMO::_sendTokensToRdpxV2Core function

Assessed type

Under/Overflow