Dopex - 0xkazim's results

Lines of code

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

Vulnerability details

impact

The perpVaultLp contract is susceptible to a flash loan attack. An attacker can exploit the vulnerability by executing flash loan transactions using both the deposit() and redeem() functions. This allows the attacker to acquire extra rdpx tokens and increase their share of assets within seconds, all without actually adding a balance to the perpVaultLP. The attacker only needs to obtain a flash loan of weth as the deposit asset, and then they can use the deposit function to mint shares and subsequently call the redeem function to receive the flash loan amount along with additional rdpx tokens.

proof of concepts

The deposit function allows users to deposit assets (weth or rdpx) into the perpVaultLp contract. Since this function is public and accessible to any users, it can be exploited by an attacker. Here's a portion of the deposit function:

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");

        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);
    }

The redeem function allows users to redeem their assets by burning their shares and receiving extra rdpx tokens.

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

        if (msg.sender != owner) {
            uint256 allowed = allowance[owner][msg.sender]; // Saves gas for limited approvals.

            if (allowed != type(uint256).max) {
                allowance[owner][msg.sender] = allowed - shares;
            }
        }
        (assets, rdpxAmount) = redeemPreview(shares);

        // Check for rounding error since we round down in previewRedeem.
        require(assets != 0, "ZERO_ASSETS");

        _rdpxCollateral -= rdpxAmount;

        beforeWithdraw(assets, shares);

        _burn(owner, shares);

        collateral.transfer(receiver, assets);

        IERC20WithBurn(rdpx).safeTransfer(receiver, rdpxAmount);

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

in this case an attacker can: Use a contract to obtain a substantial amount of weth through a flash loan.

Call the deposit function to update the _totalCollateral and mint shares corresponding to their weth balance.

In the same transaction, call the redeem function to redeem the weth, essentially paying back the flash loan. Simultaneously, the attacker receives an amount of rdpxAmount.

This attack can have a substantial impact as it effectively causes the protocol to grant the attacker a large quantity of rdpx tokens without any meaningful contribution.

Tools used

manual review

recommendation

I recommend implementing a delay mechanism for the deposited amount or restricting access to the deposit function to whitelist addresses only. These measures can help mitigate flash loan attacks and protect the protocol from such exploits

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L964 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L975-L991 https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L1002

Vulnerability details

Impact

the function addToDelegate is used to allow users to delegate weth so other users with only rdpx can use these weth by paying the required fee, however, when this function is called the totalWethDelegate will updated with the added weth. and in the other hand we have withdraw function which allows user to withdraw their weth delegated but, it did not update the totalWethDelegate which this may lead to unexpected behavior when we call the sync function to sync the rdpxV2 balance which depends on the totalWethDelegate

Proof of Concept

the function addToDelegate will update the totalWethDelegate:

 function addToDelegate(
    uint256 _amount,
    uint256 _fee
  ) external returns (uint256) {
    _whenNotPaused();
    // fee less than 100%
    _validate(_fee < 100e8, 8);
    // amount greater than 0.01 WETH
    _validate(_amount > 1e16, 4);
    // fee greater than 1%
    _validate(_fee >= 1e8, 8);

    IERC20WithBurn(weth).safeTransferFrom(msg.sender, address(this), _amount);

    Delegate memory delegatePosition = Delegate({
      amount: _amount,
      fee: _fee,
      owner: msg.sender,
      activeCollateral: 0
    });
    delegates.push(delegatePosition);

    // add amount to total weth delegated
    //@audit
    totalWethDelegated += _amount;

    emit LogAddToDelegate(_amount, _fee, delegates.length - 1);
    return (delegates.length - 1);
  }

and when we call the withdraw function the totalWethDelegated will not updated and still contains the withdraw weth:

 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);

    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;

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

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

in this case the sync function will sync the balance using the totalWethDelegated that contains the withdraw weth:

 function sync() external {
    for (uint256 i = 1; i < reserveAsset.length; i++) {
      uint256 balance = IERC20WithBurn(reserveAsset[i].tokenAddress).balanceOf(
        address(this)
      );

      if (weth == reserveAsset[i].tokenAddress) {
        balance = balance - totalWethDelegated;
      }
      reserveAsset[i].tokenBalance = balance;
    }

    emit LogSync();
  }

there is two case that can be happen in this situation:

case 1: the sync function can be called and sync the balance with incorrect data as the the totalWethDelegated containing the removed/withdraw amount of weth and this can effect the whole functionality in the rdpxV2.sol as we depend on the reserve balances too much in this contract(paying fees and burn tokens and using for bonding mechanisms too)

case 2: this can happen by Attacker by following these steps below:

1- took a huge amount of weth and calling the addToDelegate to update the totalWethDelegated with the flash loan weth(flash loan can be taken using uniswap)

2-then in the same transaction the Attacker calls the withdraw function to withdraw all his delegated weth and pay back the flashloan + the fee for the flash loan.

3- in this case the sync function may face two possibility:

A- underflow for certain amount of Time because the totalWethDelegated(e.g 1-10 million of the flashloan) may be too larger than the address.this balance.

B- the sync function will work fine without underflow but it may sync the balance with huge different between the correct totalWethDelegated(which is minus 10 million flash loan the attacker withdraw) and the wrong totalWethDelegated( which contains the removed 10m flash loan amount) and cause the whole protocol working with the incorrect balance.

the attacker only need pay the flash loan fee in the case 2.

Tools Used

manual review

Recommended Mitigation Steps

recommend to update the totalWethDelegated in the withdraw function.

 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);
   
    amountWithdrawn = delegate.amount - delegate.activeCollateral;
    _validate(amountWithdrawn > 0, 15);
    delegate.amount = delegate.activeCollateral;
+   totalWethDelegated -= amountWithdrawn
    IERC20WithBurn(weth).safeTransfer(msg.sender, amountWithdrawn);

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

Assessed type

Other

Lines of code

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

Vulnerability details

impact

In the calculateBondCost function, there is an issue with the bondDiscount calculation, which affects the precision of the required weth and rdpx values. The formula should ensure that bondDiscount has 8 decimal places, but the current implementation returns a value without the proper precision. This results in incorrect calculations for both the required weth and rdpx amounts. More details are provided in the proof of concept (POC).

proof of concepts

in the calculateBondCost function we calculate the bond discount and weth required like the follow:

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

      _validate(bondDiscount < 100e8, 14);

      rdpxRequired =
        ((RDPX_RATIO_PERCENTAGE - (bondDiscount / 2)) *
          _amount *
          DEFAULT_PRECISION) /
        (DEFAULT_PRECISION * rdpxPrice * 1e2);

      wethRequired =
        ((ETH_RATIO_PERCENTAGE - (bondDiscount / 2)) * _amount) /
        (DEFAULT_PRECISION * 1e2);

However, when calculating bondDiscount, it returns a value like 300 without the proper 8 decimal places precision. For example:

bondDiscountFactor = 30e8 sqrt(IRdpxReserve(1e18)) = sqrt(1000000000000000000) (Math.sqrt(1e18)) = (1000000000000000000)

let say ((3000000000*sqrt(1000000000000000000) * 1e2) / (1000000000000000000)) == 300 not in 8 decimal

Then we get 300, which lacks the required precision of 300e8 or 3e10. Subsequently, this value is used to calculate the required weth and rdpx, following this formula:

(((2500000000 - (300/ 2)) * 1 100000000) / (100000000 20* 100))

when:

RDPX_RATIO_PERCENTAGE = 25e8 or 2500000000 bondDiscount / 2 = 300/ 2 _amount = 1 dpxETH we need to mint rdpxPrice = 20

This results in 1249999, which is equivalent to $31249975, and this value does not accurately represent the correct amount of rdpx required for 1 DPXETH.

It's worth noting that the weth and rdpx calculations will be incorrect even if the discount factor has 8 decimals.

Tools used

manual review

recommendation

To resolve this issue, ensure that the correct formula and decimal precision are used for both the discount factor and the weth and rdpx required calculations.

Assessed type

Math