Dopex - rvierdiiev's results

Lines of code

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

Vulnerability details

Impact

PerpetualAtlanticVaultLP depositor can avoid losses by redeeminf before settle is called

Proof of Concept

PerpetualAtlanticVaultLP users deposit into the contract in order to receive funding payment as yield. When optin is exercised, that means that PerpetualAtlanticVaultLP lost funds. Then vault pays option and receives rdpx.

In order to exercise ITM options, PerpetualAtlanticVault.settle is called. This function processes several options a time. During the process it notifies PerpetualAtlanticVaultLP about loss and sends rdpx from option buyer. In this moment it means that exchange rate of PerpetualAtlanticVaultLP has decreased and vault has lost some amount of funds.

In case if arbitrum has frontrunning ability, i would simply say, that attacker can frontrun settle call and withdraw funds. But even without frontrunning it's possible to detect correct time to withdraw. Anyone can detect strike prices and amount of options for them. This allows depositor to withdraw from vault, when market price has reached strike price with big amount of options. This will allow him to get better exchange rate. After options will be settled(repaid), then he can deposit again. Such strategy can be used to earn yields, but avoid losses.

Tools Used

VsCode

Recommended Mitigation Steps

You can think about mechanism that doesn't allow to withdraw at same epoch.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

Attacker can break settling of options by sending funds to the PerpetualAtlanticVaultLP contract

Proof of Concept

When users deposits to the PerpetualAtlanticVaultLP, then _totalCollateral variable is updated, to track amount of deposited assets. In case if option is ITM, then PerpetualAtlanticVault.settle is called. This function executes option and notifies PerpetualAtlanticVaultLP about losses.

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

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

So this function should decrease _totalCollateral variable with loss amount. Also it checks, that balance of contract is exactly equal to _totalCollateral - loss. This condition is super easy to break. It's enough to send smal amount of collateral and it will revert then. Once this is done, then subtractLoss function will always revert and it will be not possible to execute options and backing of DpxEthToken will fail.

Recommended Mitigation Steps

Do not use equal comparison.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV3LiquidityAmo.sol#L324-L336

Vulnerability details

Impact

UniV3LiquidityAmo.recoverERC721 sends tokens to RdpxV2Core, which can't handle them.

Proof of Concept

UniV3LiquidityAmo.recoverERC721 function will send token from UniV3LiquidityAmo contract to the RdpxV2Core contract.

The problem is that RdpxV2Core is not designed to handle such tokens and there is no ability to send them out and as result they will be stucked there.

Tools Used

VsCode

Recommended Mitigation Steps

Add ability to use tokens in RdpxV2Core contract.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

LPs share their earned yields with new depositor.

Proof of Concept

PerpetualAtlanticVaultLP depositors provide liquidity, which is used buy PerpetualAtlanticVault. As reward they receive interests, which is accrued by calling perpetualAtlanticVault.updateFunding function.

PerpetualAtlanticVaultLP.deposit function calculates shares, they user will receive at the top of function, before all earned interests were accrued.

As result, exchange rate is incorrect, because it's actually smaller than it is in real, which makes it cheaper for depositor to receive specific amount of shares. Because of that, LPs loss some part of yields to the new depositor.

This makes it's possible for attacker to do flashloan attack in order to grab big part of rewards, when it's profitable(after repaying flash loan fees).

Tools Used

VsCode

Recommended Mitigation Steps

Accrue interests before calculating shares.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1192-L1198

Vulnerability details

Impact

RdpxV2Core.calculateBondCost function doesn't call PerpetualAtlanticVault.updateFundingPaymentPointer, which can be used by users to pay less amount of premium for the bond.

Proof of Concept

When user wants to create new bond, then he calls bond function. First, what this function will do is to calculate needed amount of rdpx and weth that user should provide. This includes premium to create option. Then required weth amount is fetched from user. Let's check how calculateBondCost works. This function calculates amount of rdpx and weth that user needs to pay and then it calculates premium that should be paid to hedge required rdpx amount. This is how premium payment is calculated. https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1192-L1198

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

The problem is that IPerpetualAtlanticVault.updateFundingPaymentPointer is not called in this case, which means that stale latestFundingPaymentPointer can be used when calling nextFundingPaymentTimestamp(). One problem with it is that in case if block.timestamp > nextFundingPaymentTimestamp(), then call will revert and will not allow user to create bond, until updateFundingPaymentPointer() will be called. Another problem is that users can call this function exactly when block.timestamp == nextFundingPaymentTimestamp() in order to calculate lower premium.

Next important thing that bond function does is purchasing of option. In this case, latestFundingPaymentPointer will be updated and premium will be calculated correctly, so core contract will pay bigger amount of premium than user has paid.

In summary, if users will purchase bond when block.timestamp == nextFundingPaymentTimestamp(), then they will have ability to use RdpxV2Core contract's weth to cover their premium.

Tools Used

VsCode

Recommended Mitigation Steps

Call updateFundingPaymentPointer function before calculating premium to calculate expiration for option correctly.

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L975-L990

Vulnerability details

Impact

RdpxV2Core.withdraw doesn't decrease totalWethDelegated, which can be used by attacker to to block using of weth

Proof of Concept

In case if someone has eth, but doesn't have rdpx, then he can deposit those funds into RdpxV2Core contract, so another person with rdpx can use it to create bond. Amount of provided funds is accumulated into totalWethDelegated variable.

When someone uses those funds, then this variable is decreased. However, when delegator withdraws his funds, then totalWethDelegated is not decreased with withdrawn amount.

Also RdpxV2Core contract has sync function which can be called by anyone. It will update balance of each asset and in case of weth, it will not count totalWethDelegated.

Attacker add one or several delegations and then withdraw, in order to decrease weth balance or make it to be 0. As result, contract will think, that all weth amount belongs to delegators and will not be able to use it, for example to provide funding.

Tools Used

VsCode

Recommended Mitigation Steps

Decrease totalWethDelegated, when delegator withdraws.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

PerpetualAtlanticVault.updateFundingDuration function will break funding.

Proof of Concept

PerpetualAtlanticVault works with epoch and each of them lasts for the fundingDuration amount of time. This variable is very important in the contract, because it is used to determine when period is finished.

LPs of the PerpetualAtlanticVaultLP vault receive funding from PerpetualAtlanticVault. This funding is calculated as amount per second and it depends on fundingDuration. So when someone will call updateFunding function, then funding will be sent to the PerpetualAtlanticVaultLP contract, according to the rate in second for the amount of seconds that have passed since last update.

Admin can call updateFundingDuration in order to change epoch duration. Once it will be changed, then rewards er second rate will be corrupted, which will make contract to sent less or more rewards to the LPs, which depends if fundingDuration is bigger or smaller than previous one.

Tools Used

VsCode

Recommended Mitigation Steps

The best solution is to pay funding for the current timestamp, according to the old rate, then recalculate funding per second, using new fundingDuration and amount that is left for previous fundingDuration.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

PerpetualAtlanticVaultLP depositors don't have ability to signal withdrawing

Proof of Concept

PerpetualAtlanticVaultLP depositors provide funds that will be used to sell options. They receive yields from premium paid for options. When user wants to withdraw, then he can call redeem function. The call will succeed only if there is enough funds in the contract.

So in case if epxEth will have success, then utilization ratio will be about 100%, which means that all funds will be used to write options. In this case users will not have ability to redeem if they want to quit. This will make people wait, when someone new deposits in order to withdraw. This creates some bad user experience.

Tools Used

VsCode

Recommended Mitigation Steps

As it is not possible to quit sold options, that means that protocol can't withdraw funds that are locked in options in any way. But protocol can have some percentage of funds(for example 5%) that are never used to write options, but serve as withdraw pool.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

Rdpx from settled options doesn't earn yields

Proof of Concept

Stakers of PerpetualAtlanticVaultLP provide weth that is used to write options. In case if option is settled, then weth is sent to the RdpxV2Core and rdpx is sent to the PerpetualAtlanticVaultLP.

Then users can redeem their shares to receive both weth and rdpx.

I believe that most of stakers in PerpetualAtlanticVaultLP would not like to manage their positions often. Because when rdpx is sent, then it can't be used to write options. User should redeem it, then swap to eth and stake eth back. Only then he will continue earn on his locked funds. But this is not convenient for users to do every time. I believe that PerpetualAtlanticVaultLP should have ability to swap all rdpx together to use funds for options again and earn premium.

Tools Used

VsCode

Recommended Mitigation Steps

Add ability to swap rdpx to weth to use it.

Assessed type

Error