Dopex - 0xWaitress's results

Lines of code

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

Vulnerability details

Impact

settle at PerpetualAtlanticVault can be permanently Dos-ed by donation due to the strict balance check requirement at VaultLP::subtractLoss.

settle at PerpetualAtlanticVault is an essential function to settle an Atlantic put option as soon as it gets into strike price/ITM, it's an important step to settle using the underlying WETH by pulling the collateralToken(aka WETH) from the VaultLP into rdpxV2Core.

  function settle(
    uint256[] memory optionIds
  )
    external
    nonReentrant
    onlyRole(RDPXV2CORE_ROLE)
    returns (uint256 ethAmount, uint256 rdpxAmount)
  {
...
// Transfer collateral token from perpetual vault to rdpx rdpxV2Core
    collateralToken.safeTransferFrom(
      addresses.perpetualAtlanticVaultLP,
      addresses.rdpxV2Core,
      ethAmount
    );

After the transfer, the function then proceeds to call subtractLoss at the VaultLP to complete the accounting so that the latest collateralToken balance is in sync with the loss incurred by the settlement.

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

However, subtractLoss at the VaultLP requires a strict balance check. A malicious user can Dos any settle tx by simply donating 1wei of collateralToken into the VaultLP such that the balanceOf the VaultLP becomes more than _totalCollateral - loss, since now _totalCollateral become out-of-sync with balanceOf. The function would revert due to failure to comply with the strict balance check.

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

Impact: permissionless attack + low cost + permanent bricking an essential function for user to settle financial vehicles = High Risk Issue

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

Proof of Concept

Copying & Modifying from tests/perp-vault/Unit.t.sol

function testSettleWithDonation() public {
    weth.mint(address(1), 1 ether);
    deposit(1 ether, address(1));

    vault.purchase(1 ether, address(this));

    uint256[] memory ids = new uint256[](1);
    ids[0] = 0;
    priceOracle.updateRdpxPrice(0.2 gwei); // initial price * 10
    uint256[] memory strikes = new uint256[](1);
    strikes[0] = 0.015 gwei;
    skip(86500); // expire
    priceOracle.updateRdpxPrice(0.010 gwei); // ITM
    //donation of weth
    weth.mint(address(vaultLp), 1 wei);
    vm.expectRevert("Not enough collateral was sent out");
    vault.settle(ids);
  }

Tools Used

Recommended Mitigation Steps

Consider: 1.simply sync the balance with _totalCollateral. 2. or use a loose form of balance check:

  function subtractLoss(uint256 loss) public onlyPerpVault {
    require(
---      collateral.balanceOf(address(this)) == _totalCollateral - loss,
+++      collateral.balanceOf(address(this)) >= _totalCollateral - loss,
---      "Not enough collateral was sent out"
+++      "LP Vault is over-subtracted"
    );
    _totalCollateral -= loss;
  }

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

totalWethDelegated is not updated during withdraw leading to potential underflow in sync function in RpdxV2Core

Consider:

1. Bob call addToDelegate with 1 WETH, totalWethDelegated=1WETH, core has 1WETH in total balance
2. Bob calls withdraw to retrieve his 1WETH. his delegate.amount is updated to his delegate.activeCollateral as 0. 1WETH is withdrawn.
3. someone calls sync(), weth's balance is updated as balance = balance - totalWethDelegated;. balance is 0 but totalWethDelegated is still 1WETH.
4. underflows causes the entire sync function bricked.

Impact:

1. other token balance would be out-of-sync upon donation since sync is broken.
2. totalWethDelegated is inflated and cannot represent the real value.
3. reLP would fail since it calls sync() of the core at the end of its execution.
4. UniV3LiquidityAmo would fail since it calls sync() at the end of an AMO operation

sync function

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

However, totalWethDelegated is not reduced during withdraw

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

Proof of Concept

Tools Used

Recommended Mitigation Steps

reduce totalWethDelegated by the amount being withdrawn in withdraw

  function withdraw(
    uint256 delegateId
  ) external returns (uint256 amountWithdrawn) {
...
+++ totalWethDelegated -= amountWithdrawn;

Assessed type

Under/Overflow

Lines of code

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

Vulnerability details

Impact

purchase of at-the-expiry option is possible creating free option opportunity for people to bond for 1 week.

purchase is a function for the rpdxV2Core to purchase a perpetual atlantic put which can be settled into the collateral whenever it's price becomes ITM. Premium is calculated based on the timeToExpiry (with ref to blackScholes model).

Here is the workflow:

1. purchase uses timeToExpiry to calculate the premium needed for buying an option, which is the difference between nextFundingPaymentTimestamp() to the current block timestamp.

2. nextFundingPaymentTimestamp = genesis + (latestFundingPaymentPointer * fundingDuration), where fundingDuration is a fixed 7 days, genesis is immutably set upon construction.

3. latestFundingPaymentPointer is updated every time the block.timestamp >= nextFundingPaymentTimestamp(). There is a possibility that block.timestamp == nextFundingPaymentTimestamp()

4. When such things happen, according to black scholes model, the premium required to purchase a put option with zero time to expiry is 0. Essentially if an attacker can always choose to bond/purchase option at this time, such that the Vault collect no premium for that initial epoch from the V2Core even it keeps writing options.

5. Although the vault would collect funding from the V2Core according to pre-defined funding rate, however, the payFunding is only callable by V2Core itself, and only triggerable by provideFunding at V2Core which is also an admin function. This control flow means if the admin forget or miss the funding call then the premium would never be collected.

  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
    uint256 strike = roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price
    IPerpetualAtlanticVaultLP perpetualAtlanticVaultLp = IPerpetualAtlanticVaultLP(
        addresses.perpetualAtlanticVaultLP
      );

    // Check if vault has enough collateral to write the options
    uint256 requiredCollateral = (amount * strike) / 1e8;

    _validate(
      requiredCollateral <= perpetualAtlanticVaultLp.totalAvailableCollateral(),
      3
    );

    uint256 timeToExpiry = nextFundingPaymentTimestamp() - block.timestamp;

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

purchase can be called by bond -> _purchaseOption at the V2Core

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

    // Compute the bond cost
    (uint256 rdpxRequired, uint256 wethRequired) = calculateBondCost(
      _amount,
      rdpxBondId
    );

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

    // update weth reserve
    reserveAsset[reservesIndex["WETH"]].tokenBalance += wethRequired;

    // purchase options
    uint256 premium;
    if (putOptionsRequired) {
      premium = _purchaseOptions(rdpxRequired);
    }
...

_purchaseOption

  function _purchaseOptions(
    uint256 _amount
  ) internal returns (uint256 premium) {
    /**
     * Purchase options and store ERC721 option id
     * Note that the amount of options purchased is the amount of rDPX received
     * from the user to sufficiently collateralize the underlying DpxEth stored in the bond
     **/
    uint256 optionId;

    (premium, optionId) = IPerpetualAtlanticVault(
      addresses.perpetualAtlanticVault
    ).purchase(_amount, address(this));

    optionsOwned[optionId] = true;
    reserveAsset[reservesIndex["WETH"]].tokenBalance -= premium;
  }

Impact: free option purchase + exercise for ITM strikes at the time of expiry possible. Since settle at RdpxV2Core can only be called by admin, thus the attack can only completed if any user called purchase of option through bondWithDelegate and the the admin settle the option within the same block (time == block.timestamp).

privileged call + severe impact = medium risk issue

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

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

Proof of Concept

Tools Used

Recommended Mitigation Steps

Consider revoking the ability to purchase at the expiry time.

Assessed type

Invalid Validation