Dopex - peakbolt's results

Lines of code

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

Vulnerability details

RdpxV2Core will purchase OTM puts that are 25% below current price during rDPX bonding. The option strike price to be purchased at is determined by the PerpetualAtlanticVault.roundUp(), which will round up the strike price to the nearest 1e6 dp.

However, an issue will occur when rDPX price is < 1e6, as roundUp() will return 1e6 even when currentPrice is < 1e6. This will cause strike price to be above current price (ITM puts), instead of below the current price (OTM puts).

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

  function purchase(
    uint256 amount,
    address to
  )
    external
    nonReentrant
    onlyRole(RDPXV2CORE_ROLE)
    returns (uint256 premium, uint256 tokenId)
  {
    _whenNotPaused();
    _validate(amount > 0, 2);

    updateFunding();

    uint256 currentPrice = getUnderlyingPrice(); // price of underlying wrt collateralToken
    //@audit - roundUp() will return 1e6 even when currentPrice is < 1e6, causing strike to be above current price
    uint256 strike = roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price
    IPerpetualAtlanticVaultLP perpetualAtlanticVaultLp = IPerpetualAtlanticVaultLP(
        addresses.perpetualAtlanticVaultLP
      );

Impact

Due to the rounding up, rDPXV2Core will purchase ITM puts instead of OTM puts, paying a much higher premium than required.

Proof of Concept

Add the following test case test/rdpxV2-core/Unit.t.sol

  function testPeakboltBond() public {
    rdpxPriceOracle.updateRdpxPrice(1e5);

    uint256 receiptTokenAmount = rdpxV2Core.bond(1 * 1e10, 0, address(this));

    (uint256 strike1, uint256 amount1, ) = vault.optionPositions(0);

    //strike price should be 0.75e5 (75% of 1e5)
    // However this will fail as it will round up to 1e6
    assertEq(strike1, 75000);
  }

Recommended Mitigation Steps

Verify that strike > rdpxPrice during purchase() to ensure the puts is OTM.

Assessed type

Math

Lines of code

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

Vulnerability details

PerpetualAtlanticVaultLP holders receives premium/funding for their deposited WETH, which are used to write PerpetualAtlanticVault put options.

However, they are able to redeem their WETH anytime as long as there are available WETH in PerpetualAtlanticVaultLP. As the losses from the options are deducted from PerpetualAtlanticVaultLP via settle(), PerpetualAtlanticVaultLP holders can evade the loss and redeem their WETH by frontrunning settle() operation. They can then backrun settle() to re-deposit the redeemed WETH at a lower share price.

Even though, Arbitrum does not have a mempool at the moment, there could possibly be MEV opportunities in the future when they decentralize the sequencer. And this issue is also applicable if the project intends to expand to other chains or make it omnichain.

Impact

The issue can allow PerpetualAtlanticVaultLP holders to arbitrage by evading options while receiving premium/funding, causing the other holders to inccur higher losses.

Proof of Concept

Imagine the following scenario,

1. Alice who is a PerpetualAtlanticVaultLP holder has deposited WETH in PerpetualAtlanticVaultLP and receives premium/funding for them.
2. Alice monitors for settle() transaction and proceed to frontrun them by calling redeem() to evade the option loss and withdraws her WETH & rDPX.
3. Alice can then backrun the settle() transactions with deposit(), to deposit the WETH & rDPX at a lower share price and continue to receive premium/funding for them.

Recommended Mitigation Steps

Add in a time lock on PerpetualAtlanticVaultLP redemption, to allow holders to request redemption and only allow them to retrieve the WETH & rDPX after a certain period of time. Note that holders should not receive fundings upon redemption request.

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVaultLP.sol#L201 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L359 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L774

Vulnerability details

PerpetualAtlanticVaultLP.subtractLoss() is used to reduce _totalCollateral in PerpetualAtlanticVaultLP when RdpxV2Core.settle() is called to settle the put options.

However, it uses a strict equality for the balanceOf() check, which means anyone can manipulate it and cause it to fail by sending 1 wei WETH to de-sync the collateral balance and _totalCollateral. When that happens, settle() will fail, preventing RdpxV2Core from realizing profits from ITM put options.

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

  function subtractLoss(uint256 loss) public onlyPerpVault {

    //@audit - this will fail when _totalCollateral is not in sync with balanceOf()
    require(
      collateral.balanceOf(address(this)) == _totalCollateral - loss,
      "Not enough collateral was sent out"
    );
    _totalCollateral -= loss;
  }

Impact

Depositors of PerpetualAtlanticVaultLP can prevent RdpxV2Core from settling any ITM put options to evade all losses. That means RdpxV2Core will suffer losses as it will not be able to realize the profits when the put options are ITM.

Proof of Concept

1. Depositor of PerpetualAtlanticVaultLP sends 1 wei of WETH to PerpetualAtlanticVaultLP. This causes PerpetualAtlanticVaultLP's collateral balance of WETH to be out of sync with _totalCollateral.
2. rDPX price drops and the put options held by rdpxV2Core are now ITM (current price below strike price) and are in profits.
3. rdpxV2Core admin tries to close the options using settle(). However, it fails, preventing it from realizing the profits.
4. Depositors of PerpetualAtlanticVaultLP continue to gain funding payments and evade the losses despite that the options are ITM.

Recommended Mitigation Steps

Use >= instead of == for the balanceOf() check in subtractLoss().

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

RdpxV2Core.emergencyWithdraw() is meant for use during an emergency (when contract is paused) to allow admin to withdraw assets from RdpxV2Core and possibly migrate to a new contract later on.

However, it only transfers out ERC20 tokens and does not withdraw the PerpetualAtlanticVault put options as those are NFTs.

Furthermore, RdpxV2Core is not able to close the OTM put options, causing the associated locked WETH to remain locked in PerpetualAtlanticVaultLP.

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

  function emergencyWithdraw(
    address[] calldata tokens
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    _whenPaused();
    IERC20WithBurn token;

    for (uint256 i = 0; i < tokens.length; i++) {
      token = IERC20WithBurn(tokens[i]);
      token.safeTransfer(msg.sender, token.balanceOf(address(this)));
    }

    emit LogEmergencyWithdraw(msg.sender, tokens);
  }

Impact

The issue will cause PerpetualAtlanticVault put options to be left in RdpxV2Core without a way to retrieve them or close them during the emergency.

Recommended Mitigation Steps

Add in withdrawals of PerpetualAtlanticVault put options NFTs in RdpxV2Core.emergencyWithdraw().

Assessed type

Other

Lines of code

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

Vulnerability details

PerpetualAtlanticVaultLP allows users to deposit WETH in exchange for shares. During deposit, perpetualAtlanticVault.updateFunding() is called to distribute the funding to PerpetualAtlanticVaultLP.

However, the issue is that previewDeposit(assets), which determines the deposit share count, is called before updateFunding(). The implication is that new depositors will receive more shares than required as the share price is lower before the funding is distributed to PerpetualAtlanticVaultLP. updateFunding() should be called before previewDeposit(assets) to update the share price before computing the share count for new deposits.

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

  function deposit(
    uint256 assets,
    address receiver
  ) public virtual returns (uint256 shares) {
    // Check for rounding error since we round down in previewDeposit.
    require((shares = previewDeposit(assets)) != 0, "ZERO_SHARES");

    //@audit this should be called before previewDeposit()
    perpetualAtlanticVault.updateFunding();

    // Need to transfer before minting or ERC777s could reenter.
    collateral.transferFrom(msg.sender, address(this), assets);

    _mint(receiver, shares);

    _totalCollateral += assets;

    emit Deposit(msg.sender, receiver, assets, shares);
  }

Impact

The issue will cause existing PerpetualAtlanticVaultLP holders to be shortchanged as new deposit receives more share than what is deposited into PerpetualAtlanticVaultLP.

Proof of Concept

When User A calls PerpetualAtlanticVaultLP.deposit(), the following will occur,

1. First, previewDeposit(assets) has determined X amount of shares to mint for the provided assets (WETH) by User A.
2. Then perpetualAtlanticVault.updateFunding() is called, which transfers the funding to PerpetualAtlanticVaultLP, increasing the WETH balance.
3. Now the WETH balance has increased, but the amount of shares to mint for User A remains the same at X. This is incorrect as share price has increase due to updateFunding() and User A should get less than X share.
4. Existing PerpetualAtlanticVaultLP holders will be shortchanged as User A takes a higher amount of shares than required.

Recommended Mitigation Steps

In PerpetualAtlanticVaultLP.deposit(), call updateFunding() before previewDeposit().

Assessed type

Other

Lines of code

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

Vulnerability details

RdpxV2Core.bond() relies on calculateBondCost() to determine the premium cost for the PerpetualAtlanticVault put option, which is purchased during bonding.

However, calculateBondCost() does not update the latestFundingPaymentPointer using updateFundingPaymentPointer() before computing timeToExpiry for calculatePremium().

When timeToExpiry == 0 at the end of the epoch, the premium calculation in calculateBondCost() will be lower than the premium calculated in PerpetualAtlanticVault.purchase(). That is because PerpetualAtlanticVault.purchase() will trigger the transition to next epoch, and the premium will be for the entire duration of the next epoch.

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

 function calculateBondCost(
    ..
    //@audit missing updateFundingPaymentPointer() here 
    uint256 timeToExpiry = IPerpetualAtlanticVault(
      addresses.perpetualAtlanticVault
    ).nextFundingPaymentTimestamp() - block.timestamp;

Impact

A user can abuse the issue to pay less premium for the put option and make RdpxV2Core pay most of the remaining amount.

Proof of Concept

1. Alice calls bond() at the end of epoch. In calculateBondCost(), the timeToExpiry will be equal to 0 as the epoch has not transition due to missing 1updateFundingPaymentPointer()`.
2. calculateBondCost() will compute premium based on timeToExpiry == 0 to derive wethRequired, which will be paid by Alice.
3. After that, PerpetualAtlanticVault.purchase() will be called to purchase the put option.
4. As PerpetualAtlanticVault.purchase() will call updateFundingPaymentPointer(), it will transition to the next epoch.
5. Within purchase(), the calculated premium will be higher as it based on entire duration of the next epoch (due to the epoch transition).
6. That means purchase() will actually transfer a higher premium amount from RdpxV2Core. So effectively Alice made RdpxV2Core pay for most of the premium.

Recommended Mitigation Steps

in calculateBondCost(), call updateFundingPaymentPointer() to set to latest epoch before computing timeToExpiry.

Assessed type

Other

Lines of code

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

Vulnerability details

PerpetualAtlanticVaultLP allows one to deposit WETH for LP shares to receive funding payments from PerpetualAtlanticVault.

However, the issue is that there is no timelock on PerpetualAtlanticVaultLP.redeem(). This will allow an attacker to steal from PerpetualAtlanticVaultLP as it is possible to increase the share price through bond() after depositing so that one can redeem at a higher share price with flash loan.

  //@audit lack of timelock on redeem allow one to deposit and redeem within same transaction
  function redeem(
    uint256 shares,
    address receiver,
    address owner
  ) public returns (uint256 assets, uint256 rdpxAmount) {

Impact

The issue allows an attacker to steal from PerpetualAtlanticVaultLP holders.

Proof of Concept

1. Alice flashloans WETH and deposits into PerpetualAtlanticVaultLP to obtain large amount of LP shares.
2. In the same transaction, Alice calls bond(), which triggers PerpetualAtlanticVault.purchase() to buy put options. Alice can specify a bond amount such that it does not lock up all the WETH in PerpetualAtlanticVaultLP, allowing Alice to withdraw after this.
3. Alice triggers updateFunding() to transfer the premium into PerpetualAtlanticVaultLP, which increases the share price.
4. Now Alice can redeem() the LP shares within the same transaction at a higher share price and repays the flash loan. The result is that Alice managed to arbitrage and profit from the redemption at the expense of the PerpetualAtlanticVaultLP holders.

To increase the profitability,

• bond() can be triggered near the end of the epoch so that the funding rate increase will be higher, as the premium will be divided by a smaller duration.

Recommended Mitigation Steps

Impose a timelock on PerpetualAtlanticVaultLP.redeem() so that it cannot be called within the same transaction as deposit().

Assessed type

Other

Lines of code

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

Vulnerability details

RdpxV2Core.withdraw() allows the delegate user to withdraw un-used WETH that was delegated.

However, withdraw() fails to reduce the totalWethDelegated by the amountWithdrawn value, causing totalWethDelegated to be incorrect.

An attacker can exploit the issue and use sync() to manipulate reserveAsset[reservesIndex["WETH"]].tokenBalance and make it zero. That will cause lowerDepeg() to fail as it requires a positive amount of reserveAsset[reservesIndex["WETH"]].tokenBalance.

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

  function withdraw(
    uint256 delegateId
  ) external returns (uint256 amountWithdrawn) {
    _whenNotPaused();
    _validate(delegateId < delegates.length, 14);
    Delegate storage delegate = delegates[delegateId];
    _validate(delegate.owner == msg.sender, 9);

    //@audit - totalWethDelegated should be reduced by amountWithdrawn after this
    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;

    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

Impact

An attacker can exploit this issue to prevent RdpxV2Core from maintaining the peg of DpxEth.

Proof of Concept

1. Flashloan X amount of WETH, where X = reserveAsset[reservesIndex["WETH"]].tokenBalance - totalWethDelegated.
2. Deposit the flashloaned X amount of WETH into RdpxV2Core using addToDelegate(). This will increase totalWethDelegated to be equal to reserveAsset[reservesIndex["WETH"]].tokenBalance.
3. Call sync() to sync asset reserves, which will then reduce reserveAsset[reservesIndex["WETH"]].tokenBalance by totalWethDelegated to zero.
4. Withdraw the delegated X amount WETH and repay flash loan.
5. lowerDepeg() will fail as it requires a positive amount of reserveAsset[reservesIndex["WETH"]].tokenBalance.

Recommended Mitigation Steps

Reduce totalWethDelegated by the amountWithdrawn during withdraw().

Assessed type

Other

Lines of code

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

Vulnerability details

settle() is used by RdpxV2Core admin to close PerpetualAtlanticVault put options and realize the profits when they are ITM.

However, it does not allow forfeiting of put options, which means there is no mechanism to unlock the associated locked WETH unless they are ITM.

There are scenarios where such a function is required, such as during an emergency or migration (transferring to a new vault version and deprecating the existing vault). In these cases, there is no mechanism to unlock the WETH associated with the OTM put options. Furthermore, in these situation, funding payment for the OTM options will likely be paused as well.

Impact

Without a mechanism to forfeit put options, the associated locked WETH will be stuck in PerpetualAtlanticVaultLP without any mechanism to release them.

Proof of Concept

scenario 1 - migration

1. An migration takes place and holders of PerpetualAtlanticVaultLP are advised to redeem and withdraw their WETH and rDPX tokens.
2. RdpxV2Core admin can only close the ITM put options and release the rDPX.
3. However, there are still WETH locked associated with the active OTM put options.
4. RdpxV2Core admin is not able to close the active OTM put options. The associated WETH remains locked for extended period of time.
5. Due to the migration, RdpxV2Core will no longer pay funding for the OTM put options as all funds are withdrawn.
6. Holders of PerpetualAtlanticVaultLP will not be able to retrieve the locked WETH associated the OTM put options. At the same time, they are also not receiving any fundings for the locked WETH.

Scenario 2 - unpausing after extended pause due to emergency

1. Suppose we have active options at strike price of 100.
2. At epoch N, price = 120 (price > strike), so option is still OTM.
3. Now the contract are paused due to an emergency.
4. At the end of epoch N, contracts are still paused so admin is not able to calculateFunding() and provideFunding() for epoch N+1. That means there are no funding payment for Epoch N+1.
5. At epoch N+1, the contracts are unpaused and resumes operation.
6. Now price has dropped to 80 (price < strike), so option is ITM.
7. Admin proceed to settle() and realize the profits from the active options.
8. Option writer then suffer a loss despite not receiving any funding payment for epoch N+1.

Recommended Mitigation Steps

Add the ability for settle() to forfeit put options.

Assessed type

Other

Lines of code

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

Vulnerability details

RdpxV2Core.bond() calls reLPContract.reLP() at the end of the bonding to adjust the LP for the rDPX/ETH pool and swap ETH for rDPX. Within reLPContract.reLP(), it first calls removeLiquidity() to remove liquidity (rDPX and ETH), followed by swapExactTokensForTokens() and addLiquidity().

However, the issue is that an attacker has control over RdpxV2Core.bond() and can use it to indirectly call reLPContract.reLP(). That means the attacker can basically frontrunned/backrunned reLP() without a mempool. It also reduces the attack cost as attacker does not pay high gas to frontrun it.

One may argue that the slippage protection is in place, but that only makes it less profitable and would not help in this case because the attacker is able to increase the trade size of the UniswapV2 transactions. That is because the attacker can adjust the bonding amount to increase the amount of liquidity removed by reLP() (and also swap/add liquidity), allowing the attacker to craft a profitable sandwich attack.

It is not recommended to allow users to have indirect control over the UniswapV2 transaction in reLPContract.

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

  function bond(
    uint256 _amount,
    uint256 rdpxBondId,
    address _to
  ) public returns (uint256 receiptTokenAmount) {

    ...
    //@audit an attacker can indirectly trigger this to perform a large trade and sandwich attack it
    // reLP
    if (isReLPActive) IReLP(addresses.reLPContract).reLP(_amount);

Impact

An attacker can steal from rdpxV2Core by crafting an profitable sandwich attack on bond().

Proof of Concept

An attacker can perform a sandwich attack as follows:

1. Perform a flash loan to borrow ETH.
2. Use the borrowed ETH to perform a ETH-to-rDPX swap on the UniswapV2 rDPX/ETH pool. This is to increase the price of rDPX before reLPContract.reLP().
3. Call rdpxV2Core.bond() to indirectly trigger reLPContract.reLP(), which will removeLiquidity(), causing it receive lesser rDPX due to the price manipulation.
4. reLPContract.reLP() will then swapExactTokensForTokens() to swap ETH-to-rDPX using ETH from removed liquidity. This further increase price of rDPX.
5. addLiquidity() is then called by reLPContract.reLP() to return part of the liquidity into the pool.
6. Attacker now swap rDPX back to ETH to realize profit from the attack and pay back the flash loan.

Recommended Mitigation Steps

Remove reLPContract.ReLP() from bond() and trigger separately to prevent user access to it.

Assessed type

Uniswap

Lines of code

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

Vulnerability details

fundingDuration is used to determine the time period between funding payments for the active put options. It is also used within PerpetualAtlanticVault.calculateFunding() to determine the start time of the epoch and compute the funding payment for the next epoch.

However, the calculateFunding() will be affected if updateFundingDuration() is used by the admin to change fundingDuration. The result is that the funding payment will be higher/lower causing an incorrect payment to be paid to PerpetualAtlanticVaultLP when fundingDuration is changed.

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

  function calculateFunding(
    ...
    //@audit fundingDuration is used to compute the start time of the epoch and derive timeToExpiry.
      uint256 timeToExpiry = nextFundingPaymentTimestamp() -
        (genesis + ((latestFundingPaymentPointer - 1) * fundingDuration));

    //@audit timeToExpiry is a parameter for premium calculation
      uint256 premium = calculatePremium(
        strike,
        amount,
        timeToExpiry,
        getUnderlyingPrice()
      );

Impact

Changing fundingDuration will affect the funding payment computation, causing PerpetualAtlanticVaultLP holders to be underpaid or overpaid.

Proof of Concept

In calculateFunding(), the premium calculation is determined by timeToExpiry below.

      uint256 timeToExpiry = nextFundingPaymentTimestamp() -
        (genesis + ((latestFundingPaymentPointer - 1) * fundingDuration));

timeToExpiry is derived from the time difference between nextFundingPaymentTimestamp() and the start time. Note that start time computation is derived by this formula: (genesis + ((latestFundingPaymentPointer - 1) * fundingDuration)).

Now lets suppose fundingDuration is changed from 7 to 5 days for the current epoch.

The start time computation will then be incorrect as it uses the new 5 days fundingDuration instead of 7 days to determine the start time. That will cause start time to be much earlier, leading to a longer timeToExpiry, which causes premium to be much higher.

The result is that PerpetualAtlanticVaultLP holders will be overpaid with a higher premium during that time period.

Recommended Mitigation Steps

Make fundingDuration immutable and remove updateFundingDuration().

Assessed type

Other

Lines of code

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

Vulnerability details

During rDPX bonding, rdpxV2Core will purchase put options for the rDPX that are used for minting DPXETH. This is for hedging against rDPX price drop to protect the DPXETH peg. The amount of put options to purchase is equivalent to the amount of rDPX that are used for minting DPXETH, which is rdxRequired as given by calculateBondCost().

The issue is that for regular bonding, rdxRequired is reduced by the discount provided by Treasury Reserve and the amount of rDPX used for minting is actually rdxRequired + rDPX discount given by Treasury Reserve. We can see the _transfer() code below actually increment the rDPX Backing Reserve by _rdpxAmount + extraRdpxToWithdraw, which is the actual amount of rDPX use for minting DPXETH.

So users that perform regular/delegate bonding will pay less premium due to the reduced amount of put options. With a reduced amount of put options, the hedging in place to protect DPXETH peg will be less effective.

Impact

Users who performs bonding via rDPX Decaying Bond will be shortchanged as they are paying the full premium while users who perform regular/delegate bonding will pay less premium than required.

Also, with a reduced amount of put options, it will reduce effectiveness of the hedging against rDPX price drop, thereby leading to higher likelihood of DPX ETH depeg.

Proof of Concept

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

  function bond(
    uint256 _amount,
    uint256 rdpxBondId,
    address _to
  ) public returns (uint256 receiptTokenAmount) {
    _whenNotPaused();
    // Validate amount
    _validate(_amount > 0, 4);

    //@audit rdpxRequired is reduced by discount for regular bonding
    // Compute the bond cost
    (uint256 rdpxRequired, uint256 wethRequired) = calculateBondCost(
      _amount,
      rdpxBondId
    );

    //@audit for regular bonding, it should purchase rdpxRequired + rdpx discount
    // purchase options
    uint256 premium;
    if (putOptionsRequired) {
      premium = _purchaseOptions(rdpxRequired);
    }

    //@audit we can see the _transfer() code below actually uses _rdpxAmount + extraRdpxToWithdraw
     _transfer(rdpxRequired, wethRequired - premium, _amount, rdpxBondId);

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

  function calculateBondCost(
    uint256 _amount,
    uint256 _rdpxBondId
  ) public view returns (uint256 rdpxRequired, uint256 wethRequired) {
    uint256 rdpxPrice = getRdpxPrice();

    if (_rdpxBondId == 0) {
      uint256 bondDiscount = (bondDiscountFactor *
        Math.sqrt(IRdpxReserve(addresses.rdpxReserve).rdpxReserve()) *
        1e2) / (Math.sqrt(1e18)); // 1e8 precision

      _validate(bondDiscount < 100e8, 14);

      //@audit for regular bonding rdpxRequired is reduced by discount
      rdpxRequired =
        ((RDPX_RATIO_PERCENTAGE - (bondDiscount / 2)) *
          _amount *
          DEFAULT_PRECISION) /
        (DEFAULT_PRECISION * rdpxPrice * 1e2);

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L681-L684
  function _transfer(
    uint256 _rdpxAmount,
    uint256 _wethAmount,
    uint256 _bondAmount,
    uint256 _bondId
  ) internal {

    //@audit amount of rDPX used for minting is _rdpxAmount + extraRdpxToWithdraw due to discount
      reserveAsset[reservesIndex["RDPX"]].tokenBalance +=
        _rdpxAmount +
        extraRdpxToWithdraw;
    }

Recommended Mitigation Steps

Update _purchaseOptions(rdpxRequired) to include the discount provided by Treasury Reserve.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L286-L289 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1196-L1197

Vulnerability details

rDPX bonding requires users to provide rDPX tokens and WETH to mint DPXETH. In addition, users have to pay premium for RdpxV2Core to purchase PerpetualAtlanticVault put options.

The issue is that the option premium cost is dependent on timeToExpiry, so users will pay more when the timeToExpiry is higher.

This is an unfair cost computation as users who perform bonding at the start of the epoch are charged more premium than users who performed bonding for the same quantity of bonds near the end of the epoch. This is true even when rDPX price remains the same.

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

  function purchase(
    ...
    // Get total premium for all options being purchased
    premium = calculatePremium(strike, amount, timeToExpiry, 0);

    //@audit during the start of epoch (higher timeToExpiry), users pay more premium 
    // Transfer premium from msg.sender to PerpetualAtlantics vault
    collateralToken.safeTransferFrom(msg.sender, address(this), premium);

Impact

The issue will cause users that bond near the start of epoch to be shortchanged as they will pay more than users that bond near the end of the epoch.

Proof of Concept

Imagine the following scenario, First we suppose rDPX price remains the same throughput, to keep it simple.

1. User A calls bond() for N amount of bond at the start of the option epoch. User A is charged X amount of premium.
2. User B calls bond() for same N amount of bond at the end the option epoch. However, User B pay less than X premium for the bonding. So User A is charged more despite receiving the same amount of bond and providing the same rDPX amount.

Recommended Mitigation Steps

Consider using a fixed fee approach for bond() to cover the premium payment so that users pay a consistent amount at different part of the epoch.

Assessed type

Other

Lines of code

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

Vulnerability details

PerpetualAtlanticVaultLP.redeem() allow LP holders to redeem their LP shares in exchange for an equivalent amount of WETH and rDPX tokens.

However, the issue is that redeem() does not allow redemption when WETH balance in PerpetualAtlanticVaultLP is zero, even when rDPX balance is positive.

This could occur when all the WETH in PerpetualAtlanticVaultLP are used up to settle the ITM options and buy the rDPX tokens from RdpxV2Core. In that situation, rDPX balance in PerpetualAtlanticVaultLP will be positive as rDPX is purchased from the option settlement while WETH balance is zero.

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

  function redeem(
    uint256 shares,
    address receiver,
    address owner
  ) public returns (uint256 assets, uint256 rdpxAmount) {
    ...

    (assets, rdpxAmount) = redeemPreview(shares);

    //@audit in the event that all the WETH are used up, this will prevent LP holders from redeeming the remaining rDPX.
    // Check for rounding error since we round down in previewRedeem.
    require(assets != 0, "ZERO_ASSETS");

Impact

LP holders of PerpetualAtlanticVaultLP will not be able to redeem their shares when the issue occur.

Proof of Concept

Suppose the following situation,

1. All deposited WETH in PerpetualAtlanticVaultLP are fully locked as they are used to purchase put options and any un-used WETH are redeemed by users. So no there no unlocked WETH at this moment.
2. Now rDPX experience a significant price drop and all active put options are ITM.
3. RdpxV2Core admin proceed to settle all the options.
4. Due to the settlement, all the locked WETH in PerpetualAtlanticVaultLP are then transferred to RdpxV2Core in exchange for rDPX tokens.
5. Now the WETH balance is zero while rDPX balance is positive in PerpetualAtlanticVaultLP. However, due to the require(assets != 0) check, PerpetualAtlanticVaultLP holders are not able to redeem their shares.

Recommended Mitigation Steps

Change to require(assets != 0 || rdpxAmount != 0, "ZERO_ASSETS");

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L160-L178

Vulnerability details

UniV2LiquidityAmo._sendTokensToRdpxV2Core() is called to send all ETH and rDPX tokens in UniV2LiquidityAmo to RdpxV2Core after liquidity/swap operations.

However, it is missing a call to RdpxV2Core.sync() to synchronize the tokenBalance for the ETH and rDPX reserves. This will cause a de-sync in the tokenBalance and affect the operations in RdpxV2Core.

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L160-L178

  function _sendTokensToRdpxV2Core() internal {
    uint256 tokenABalance = IERC20WithBurn(addresses.tokenA).balanceOf(
      address(this)
    );
    uint256 tokenBBalance = IERC20WithBurn(addresses.tokenB).balanceOf(
      address(this)
    );
    // transfer token A and B from this contract to the rdpxV2Core
    IERC20WithBurn(addresses.tokenA).safeTransfer(
      addresses.rdpxV2Core,
      tokenABalance
    );
    IERC20WithBurn(addresses.tokenB).safeTransfer(
      addresses.rdpxV2Core,
      tokenBBalance
    );

    emit LogAssetsTransfered(msg.sender, tokenABalance, tokenBBalance);
  }

Impact

The issue will cause a de-sync of ETH and rDPX token balance in RdpxV2Core, leading to DoS of certain operations that adjust token balance such as settle(), bond(), provideFunding().

Recommended Mitigation Steps

Add a call to RdpxV2Core.sync() in UniV2LiquidityAmo._sendTokensToRdpxV2Core().

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L286-L295

Vulnerability details

ReLPContract.reLP() is used to adjust the LP allocation in the UniswapV2 rDPX/ETH pool by removing part of liquidity, swapping received ETH to rDPX and re-adding liquidity. It essentially increase price of rDPX and increase value of rDPX backing reserve.

However, when re-adding liquidity back to the pool, it does not handle the case where there could be un-used ETH balance after addLiquidity() is called. It is possible for ETH to be partially utilized for addLiquidity() due to the 0.3% trading fee for swap.

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/reLP/ReLPContract.sol#L286-L295

  function reLP(uint256 _amount) external onlyRole(RDPXV2CORE_ROLE) {
    ...

    //@audit tokenAAmountOut has 0.03% fee deducted
    uint256 tokenAAmountOut = IUniswapV2Router(addresses.ammRouter)
      .swapExactTokensForTokens(
        amountB / 2,
        mintokenAAmount,
        path,
        address(this),
        block.timestamp + 10
      )[path.length - 1];


    (, , uint256 lp) = IUniswapV2Router(addresses.ammRouter).addLiquidity(
      addresses.tokenA,
      addresses.tokenB,
      tokenAAmountOut,
      amountB / 2, //@audit this will be partially utilized as actual amt is derived from tokenAAmountOut
      0,
      0,
      address(this),
      block.timestamp + 10
    );

    ...
    //@audit missing transfer of balance ETH in reLPContract
    // transfer rdpx to rdpxV2Core
    IERC20WithBurn(addresses.tokenA).safeTransfer(
      addresses.rdpxV2Core,
      IERC20WithBurn(addresses.tokenA).balanceOf(address(this))
    );
    IRdpxV2Core(addresses.rdpxV2Core).sync();

If we see UniswapV2Router._addLiquidity(), it will calculate the amountB based on amountADesired. So the amount of ETH used for addLiquidity() will be lower. See POC for more explanation.

https://github.com/Uniswap/v2-periphery/blob/master/contracts/UniswapV2Router02.sol#L49-L53

    function _addLiquidity(
        address tokenA,
        address tokenB,
        uint amountADesired,
        uint amountBDesired,
        uint amountAMin,
        uint amountBMin
    ) internal virtual returns (uint amountA, uint amountB) {
        // create the pair if it doesn't exist yet
        if (IUniswapV2Factory(factory).getPair(tokenA, tokenB) == address(0)) {
            IUniswapV2Factory(factory).createPair(tokenA, tokenB);
        }
        (uint reserveA, uint reserveB) = UniswapV2Library.getReserves(factory, tokenA, tokenB);
        if (reserveA == 0 && reserveB == 0) {
            (amountA, amountB) = (amountADesired, amountBDesired);
        } else {
            uint amountBOptimal = UniswapV2Library.quote(amountADesired, reserveA, reserveB);
            if (amountBOptimal <= amountBDesired) {
                require(amountBOptimal >= amountBMin, 'UniswapV2Router: INSUFFICIENT_B_AMOUNT');
                (amountA, amountB) = (amountADesired, amountBOptimal);
            } else {

Impact

Un-used balance of ETH is stuck within the ReLPContract.

Proof of Concept

Here is a simple scenario to explain a possible issue,

• ReLP() calls swapExactTokensForTokens() to swap amountB / 2 ETH and obtain tokenAAmountOut of rDPX tokens. As we are providing exactly amountB / 2 of ETH for swapExactTokensForTokens(), the received tokenAAmountOut will be deducted of the fee.

• To keep it simple, we assume an exchange rate of 1 ETH to 1 rDPX.

• Suppose for the swapExactTokensForTokens(), we swap amountB/2 = 100 ETH and we will receive tokenAAmountOut = 99.7 rDPX (fee paid is 0.3% of 100 rDPX = 0.03 rDPX)

• Now if we provide 99.7 rDPX (amountADesired) and 100 ETH (amountBDesired) for addLiquidity(), only 99.7 ETH will be used as _addLiquidity() will calculate the amountB based on amountADesired. Based on 1:1 exchange rate that will be 99.7ETH (See code above).

• So after addLiquidity(), 0.03 ETH will be left within the RELPContract.

For more details, run the following test case in tests/rdpxV2-core/Periphery.t.sol

function testPeakboltReLpContract() public {
    testV2Amo();

    // set address in reLP contract and grant role
    reLpContract.setAddresses(
      address(rdpx),
      address(weth),
      address(pair),
      address(rdpxV2Core),
      address(rdpxReserveContract),
      address(uniV2LiquidityAMO),
      address(rdpxPriceOracle),
      address(factory),
      address(router)
    );
    reLpContract.grantRole(reLpContract.RDPXV2CORE_ROLE(), address(rdpxV2Core));

    reLpContract.setreLpFactor(9e4);

    // add liquidity
    uniV2LiquidityAMO.addLiquidity(5e18, 1e18, 0, 0);
    uniV2LiquidityAMO.approveContractToSpend(
      address(pair),
      address(reLpContract),
      type(uint256).max
    );

    rdpxV2Core.setIsreLP(true);

    uint256 reLpwethBalanceBefore = weth.balanceOf(address(reLpContract));

    rdpxV2Core.bond(1 * 1e18, 0, address(this));

    uint256 reLpwethBalanceRemaining = weth.balanceOf(address(reLpContract)) - reLpwethBalanceBefore;
    console.log("Left over WETH stuck in reLPContract = %d", reLpwethBalanceRemaining);
  }

Recommended Mitigation Steps

Transfer any remaining balance of ETH back to rdpxV2Core and call sync().

Assessed type

Uniswap

Lines of code

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

Vulnerability details

Users can delegate their WETH and earn fees when its utilized by others to bond with their rDPX tokens.

However, the delegates are not able to withdraw un-used WETH when paused. This is not fair to the delegates as bondWithDelegate() is disabled when paused so there is no opportunity for delegates to earn fees on their delegated WETH.

Proof of Concept

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

  function withdraw(
    uint256 delegateId
  ) external returns (uint256 amountWithdrawn) {
        //@audit this prevent withdraw of delegated WETH when paused
    _whenNotPaused();

Recommended Mitigation Steps

Do not disable withdraw() when paused to allow WETH delegate withdraw their un-used WETH.

Assessed type

Other