Dopex - said's results

Lines of code

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

Vulnerability details

Impact

When PerpetualAtlanticVaultLP.subtractLoss is triggered, it will check if collateral balance inside the vault LP is equal to _totalCollateral - loss. This check is too restrictive and prone to DoS.

Proof of Concept

PerpetualAtlanticVaultLP.subtractLoss is called by perp vault, providing the loss and the check is performed.

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

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

Attacker can easily transfer dust amount of collateral and break this function. This will cause PerpetualAtlanticVault.settle to revert every time.

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

Foundry PoC :

Add this test to Unit contract inside /tests/rdpxV2-core/Unit.t.sol :

    function testSettleRevert() public {
        address alice = makeAddr("alice");
        weth.mint(alice, 1e18);
        // this one bought at price 2e7
        rdpxV2Core.bond(5 * 1e18, 0, address(this));
        // price change so bought will have different options price
        rdpxPriceOracle.updateRdpxPrice(3e7);
        rdpxV2Core.bond(1 * 1e18, 0, address(this));

        vault.addToContractWhitelist(address(rdpxV2Core));
        uint256[] memory _ids = new uint256[](1);

        _ids[0] = 0;
        rdpxPriceOracle.updateRdpxPrice(15e6);
        // before settle is called, alice send weth to vault lp
        vm.prank(alice);
        weth.transfer(address(vaultLp), 1);
        // settle option
        vm.expectRevert("Not enough collateral was sent out");
        rdpxV2Core.settle(_ids);
    }

Run the test :

forge test --match-contract Unit --match-test testSettleRevert -vvv

Tools Used

Manual review

Recommended Mitigation Steps

Since subtractLoss only called by perp vault, it is safe to delete the check :

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

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

Due to wrong order between previewDeposit and updateFunding inside PerpetualAtlanticVaultLP.deposit. In some case, user can get immediate profit when call deposit and redeem in the same block.

Proof of Concept

When deposit is called, first previewDeposit will be called to get the shares based on assets provided.

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

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

Inside previewDeposit, it will call convertToShares to calculate the shares.

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

  function previewDeposit(uint256 assets) public view returns (uint256) {
    return convertToShares(assets);
  }

convertToShares calculate shares based on the provided assets, supply and totalVaultCollateral. _totalCollateral is also part of totalVaultCollateral that will be used inside the calculation.

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

  function convertToShares(
    uint256 assets
  ) public view returns (uint256 shares) {
    uint256 supply = totalSupply;
    uint256 rdpxPriceInAlphaToken = perpetualAtlanticVault.getUnderlyingPrice();

    uint256 totalVaultCollateral = totalCollateral() +
      ((_rdpxCollateral * rdpxPriceInAlphaToken) / 1e8);
    return
      supply == 0 ? assets : assets.mulDivDown(supply, totalVaultCollateral);
  }

After the shares calculation, perpetualAtlanticVault.updateFunding will be called, this function will send collateral to vault LP if conditions are met and increase _totalCollateral.

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

  function updateFunding() public {
    updateFundingPaymentPointer();
    uint256 currentFundingRate = fundingRates[latestFundingPaymentPointer];
    uint256 startTime = lastUpdateTime == 0
      ? (nextFundingPaymentTimestamp() - fundingDuration)
      : lastUpdateTime;
    lastUpdateTime = block.timestamp;

>>> collateralToken.safeTransfer(
      addresses.perpetualAtlanticVaultLP,
      (currentFundingRate * (block.timestamp - startTime)) / 1e18
    );

>>> IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).addProceeds(
      (currentFundingRate * (block.timestamp - startTime)) / 1e18
    );

    emit FundingPaid(
      msg.sender,
      ((currentFundingRate * (block.timestamp - startTime)) / 1e18),
      latestFundingPaymentPointer
    );
  }

It means if _totalCollateral is increased, user can get immediate profit when they call redeem.

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

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

When redeemPreview is called and trigger _convertToAssets, it will used this newly increased _totalCollateral.

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

  function _convertToAssets(
    uint256 shares
  ) internal view virtual returns (uint256 assets, uint256 rdpxAmount) {
    uint256 supply = totalSupply;
    return
      (supply == 0)
        ? (shares, 0)
        : (
>>>       shares.mulDivDown(totalCollateral(), supply),
          shares.mulDivDown(_rdpxCollateral, supply)
        );
  }

This will open sandwich and MEV attack opportunity inside vault LP.

Foundry PoC :

Add this test to Unit contract inside /tests/rdpxV2-core/Unit.t.sol, also add import "forge-std/console.sol"; in the contract :

    function testSandwichProvideFunding() public {
        rdpxV2Core.bond(20 * 1e18, 0, address(this));
        rdpxV2Core.bond(20 * 1e18, 0, address(this));
        skip(86400 * 7);
        vault.addToContractWhitelist(address(rdpxV2Core));
        vault.updateFundingPaymentPointer();

        // test funding succesfully
        uint256[] memory strikes = new uint256[](1);
        strikes[0] = 15e6;
        // calculate funding is done properly
        vault.calculateFunding(strikes);

        uint256 funding = vault.totalFundingForEpoch(
            vault.latestFundingPaymentPointer()
        );

        // send funding to rdpxV2Core and call sync
        weth.transfer(address(rdpxV2Core), funding);
        rdpxV2Core.sync();
        rdpxV2Core.provideFunding();
        skip(86400 * 6);
        uint256 balanceBefore = weth.balanceOf(address(this));
        console.log("balance of eth before deposit and redeem:");
        console.log(balanceBefore);
        weth.approve(address(vaultLp), type(uint256).max);
        uint256 shares = vaultLp.deposit(1e18, address(this));
        vaultLp.redeem(shares, address(this), address(this));
        uint256 balanceAfter = weth.balanceOf(address(this));
        console.log("balance after deposit and redeem:");
        console.log(balanceAfter);
        console.log("immediate profit :");
        console.log(balanceAfter - balanceBefore);
    }

Run the test :

forge test --match-contract Unit --match-test testSandwichProvideFunding -vvv

Log Output :

Logs:
  balance of eth before deposit and redeem:
  18665279470073000000000

  balance after deposit and redeem:
  18665299797412715619861

  immediate profit :
  20327339715619861

Tools Used

Manual Review

Recommended Mitigation Steps

Move perpetualAtlanticVault.updateFunding before previewDeposit is calculated.

  function deposit(
    uint256 assets,
    address receiver
  ) public virtual returns (uint256 shares) {
+    perpetualAtlanticVault.updateFunding();
    // 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);
  }

Assessed type

MEV

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L481-L483 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L286 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L539-L551

Vulnerability details

Impact

when putOptionsRequired is true, there is period of time where bond operations will always revert when try to purchase options from perp atlantic vault.

Proof of Concept

When bond is called and putOptionsRequired is true, it will call _purchaseOptions providing the calculated rdpxRequired.

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

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

    _transfer(rdpxRequired, wethRequired - premium, _amount, rdpxBondId);

    // Stake the ETH in the ReceiptToken contract
    receiptTokenAmount = _stake(_to, _amount);

    // reLP
    if (isReLPActive) IReLP(addresses.reLPContract).reLP(_amount);

    emit LogBond(rdpxRequired, wethRequired, receiptTokenAmount);
  }

Inside _purchaseOptions, it will call PerpetualAtlanticVault.purchase providing the amount and address(this) as receiver :

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

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

Then inside purchase, it will calculate premium using calculatePremium function, providing strike, amount, timeToExpiry.

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

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

    // ... rest of operations
  }

Inside this calculatePremium, it will get price from option pricing :

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

  function calculatePremium(
    uint256 _strike,
    uint256 _amount,
    uint256 timeToExpiry,
    uint256 _price
  ) public view returns (uint256 premium) {
    premium = ((IOptionPricing(addresses.optionPricing).getOptionPrice(
      _strike,
      _price > 0 ? _price : getUnderlyingPrice(),
      getVolatility(_strike),
      timeToExpiry
    ) * _amount) / 1e8);
  }

The provided OptionPricingSimple.getOptionPrice will use Black-Scholes model to calculate the price :

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/libraries/BlackScholes.sol#L33-L89

  function calculate(
    uint8 optionType,
    uint256 price,
    uint256 strike,
    uint256 timeToExpiry,
    uint256 riskFreeRate,
    uint256 volatility
  ) internal pure returns (uint256) {
    bytes16 S = ABDKMathQuad.fromUInt(price);
    bytes16 X = ABDKMathQuad.fromUInt(strike);
    bytes16 T = ABDKMathQuad.div(
      ABDKMathQuad.fromUInt(timeToExpiry),
      ABDKMathQuad.fromUInt(36500) // 365 * 10 ^ DAYS_PRECISION
    );
    bytes16 r = ABDKMathQuad.div(
      ABDKMathQuad.fromUInt(riskFreeRate),
      ABDKMathQuad.fromUInt(10000)
    );
    bytes16 v = ABDKMathQuad.div(
      ABDKMathQuad.fromUInt(volatility),
      ABDKMathQuad.fromUInt(100)
    );
    bytes16 d1 = ABDKMathQuad.div(
      ABDKMathQuad.add(
        ABDKMathQuad.ln(ABDKMathQuad.div(S, X)),
        ABDKMathQuad.mul(
          ABDKMathQuad.add(
            r,
            ABDKMathQuad.mul(v, ABDKMathQuad.div(v, ABDKMathQuad.fromUInt(2)))
          ),
          T
        )
      ),
      ABDKMathQuad.mul(v, ABDKMathQuad.sqrt(T))
    );
    bytes16 d2 = ABDKMathQuad.sub(
      d1,
      ABDKMathQuad.mul(v, ABDKMathQuad.sqrt(T))
    );
    if (optionType == OPTION_TYPE_CALL) {
      return
        ABDKMathQuad.toUInt(
          ABDKMathQuad.mul(
            _calculateCallTimeDecay(S, d1, X, r, T, d2),
            ABDKMathQuad.fromUInt(DIVISOR)
          )
        );
    } else if (optionType == OPTION_TYPE_PUT) {
      return
        ABDKMathQuad.toUInt(
          ABDKMathQuad.mul(
            _calculatePutTimeDecay(X, r, T, d2, S, d1),
            ABDKMathQuad.fromUInt(DIVISOR)
          )
        );
    } else return 0;
  }

The problem lies inside calculatePremium due to not properly check if current time less than 864 seconds (around 14 minutes). because getOptionPrice will use time expiry in days multiply by 100.

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/libraries/OptionPricingSimple.sol#L72

uint256 timeToExpiry = (expiry * 100) / 86400;

If nextFundingPaymentTimestamp() - block.timestamp is less than 864 seconds, it will cause timeToExpiry inside option pricing is 0 and the call will always revert. This will cause an unexpected revert period around 14 minutes every funding epoch (in this case every week).

Foundry PoC :

Try to simulate calculatePremium when nextFundingPaymentTimestamp() - block.timestamp is less than 864 seconds.

Add this test to Unit contract inside /tests/rdpxV2-core/Unit.t.sol, also add import "forge-std/console.sol"; and import {OptionPricingSimple} from "contracts/libraries/OptionPricingSimple.sol"; in the contract :

    function testOptionPricingRevert() public {
        OptionPricingSimple optionPricingSimple;
        optionPricingSimple = new OptionPricingSimple(100, 5e6);

        (uint256 rdpxRequired, uint256 wethRequired) = rdpxV2Core
            .calculateBondCost(1 * 1e18, 0);

        uint256 currentPrice = vault.getUnderlyingPrice(); // price of underlying wrt collateralToken
        uint256 strike = vault.roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price
        // around 14 minutes before next funding payment
        vm.warp(block.timestamp + 7 days - 863 seconds);
        uint256 timeToExpiry = vault.nextFundingPaymentTimestamp() -
            block.timestamp;
        console.log("What is the current price");
        console.log(currentPrice);
        console.log("What is the strike");
        console.log(strike);
        console.log("What is time to expiry");
        console.log(timeToExpiry);
        uint256 price = vault.getUnderlyingPrice();
        // will revert
        vm.expectRevert();
        optionPricingSimple.getOptionPrice(strike, price, 100, timeToExpiry);
    }

Tools Used

Manual review

Recommended Mitigation Steps

Set minimum timeToExpiry inside calculatePremium :

  function calculatePremium(
    uint256 _strike,
    uint256 _amount,
    uint256 timeToExpiry,
    uint256 _price
  ) public view returns (uint256 premium) {
    premium = ((IOptionPricing(addresses.optionPricing).getOptionPrice(
      _strike,
      _price > 0 ? _price : getUnderlyingPrice(),
      getVolatility(_strike),
-      timeToExpiry
+      timeToExpiry < 864 ? 864 : timeToExpiry
    ) * _amount) / 1e8);
  }

Assessed type

Timing

Lines of code

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

Vulnerability details

Impact

When user add delegated eth via addToDelegate, it will increase totalWethDelegated. However, when the delegated eth is pulled by user via withdraw, it is not updating totalWethDelegated and will break RdpxV2Core accounting.

Proof of Concept

When addToDelegate is called, it will increase totalWethDelegated based on _amount input.

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

  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
>>> totalWethDelegated += _amount;

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

But when is called withdraw called, totalWethDelegated is not updated.

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

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

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

    emit LogDelegateWithdraw(delegateId, amountWithdrawn);
  }

This will break RdpxV2Core weth reserve accounting next time user call sync, making accounting of weth reserve less than it should be.

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

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

Foundry PoC :

Add this test to Unit contract inside /tests/rdpxV2-core/Unit.t.sol, also add import "forge-std/console.sol"; in the contract :

    function testWithdrawBreakAccounting() public {
        // Bond token so weth inside core is not empty, prevent underflow
        rdpxV2Core.bond(5 * 1e18, 0, address(this));
        console.log("balance of weth inside V2Core before delegate");
        console.log(weth.balanceOf(address(rdpxV2Core)));
        (, uint256 wethReserve1, ) = rdpxV2Core.getReserveTokenInfo("WETH");
        console.log("accounting of weth reserve inside V2Core before delegate");
        console.log(wethReserve1);
        // delegate
        rdpxV2Core.addToDelegate(1 * 1e18, 10e8);
        console.log("balance of weth inside V2Core after delegate");
        console.log(weth.balanceOf(address(rdpxV2Core)));
        (, wethReserve1, ) = rdpxV2Core.getReserveTokenInfo("WETH");
        console.log("accounting of weth reserve inside V2Core after delegate");
        console.log(wethReserve1);
        // withdraw
        rdpxV2Core.withdraw(0);
        rdpxV2Core.sync();
        console.log("balance of weth inside V2Core after withdraw and sync");
        console.log(weth.balanceOf(address(rdpxV2Core)));
        (, wethReserve1, ) = rdpxV2Core.getReserveTokenInfo("WETH");
        console.log(
            "accounting of weth reserve inside V2Core after withdraw and sync"
        );
        console.log(wethReserve1);
    }

Run the test :

forge test --match-contract Unit --match-test testWithdrawBreakAccounting -vvv

Log ouput :

  balance of weth inside V2Core before delegate
  1225000000000000000
  accounting of weth reserve inside V2Core before delegate
  1225000000000000000

  balance of weth inside V2Core after delegate
  2225000000000000000
  accounting of weth reserve inside V2Core after delegate
  1225000000000000000

  balance of weth inside V2Core after withdraw and sync
  1225000000000000000
  accounting of weth reserve inside V2Core after withdraw and sync
  225000000000000000

It can be observed that the accounting of weth token balance is less than the actual balance inside the contract.

Tools Used

Manual Review

Recommended Mitigation Steps

Decrease totalWethDelegated when withdraw is called :

  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

Error

Lines of code

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

Vulnerability details

Impact

reLP is the process by which part of the Uniswap V2 liquidity is removed, then the ETH received is swapped to rDPX. This essentially increases the price of rDPX and also increases the Backing Reserve of rDPX. However,reLP's tokenAToRemove calculated using wrong formula when reLP is performed, this will lead to wrong amount will be processed by this operations and this could make reLP failed to reach its goal.

Proof of Concept

In the docs, these are the the formula to calculate rDPX to remove:

((amount_lp * 4) / rdpx_supply) * lp_rdpx_reserves * base_relp_percent

where

base_relp_percent = Math.sqrt(reserves_rdpx) * relp_factor

Inside the code, it is implemented incorrectly.

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

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

    uint256 baseReLpRatio = (reLPFactor *
      Math.sqrt(tokenAInfo.tokenAReserve) *
      1e2) / (Math.sqrt(1e18)); // 1e6 precision

    uint256 tokenAToRemove = ((((_amount * 4) * 1e18) /
>>>   tokenAInfo.tokenAReserve) *
      tokenAInfo.tokenALpReserve *
      baseReLpRatio) / (1e18 * DEFAULT_PRECISION * 1e2);

    // ...
  }

It can be observed that instead of using rdpx supply, it used tokenAReserve when divide ((_amount * 4) * 1e18).

Tools Used

Manual review

Recommended Mitigation Steps

Correct the calculation using the documented formula :

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

    uint256 baseReLpRatio = (reLPFactor *
      Math.sqrt(tokenAInfo.tokenAReserve) *
      1e2) / (Math.sqrt(1e18)); // 1e6 precision

    uint256 tokenAToRemove = ((((_amount * 4) * 1e18) /
-      tokenAInfo.tokenAReserve) *
+      IERC20WithBurn(addresses.tokenA).totalSupply()) *
      tokenAInfo.tokenALpReserve *
      baseReLpRatio) / (1e18 * DEFAULT_PRECISION * 1e2);


    // ...
  }
```diff


## Assessed type

Error

Lines of code

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

Vulnerability details

Impact

Inside reLP operation, one of the crucial step is swapping token B returned from removing liquidity to token A before adding again to the pool. However, there is a mistake when calculating mintokenAAmount. This could result in wrong min amount and could lead to unexpected result when performing the whole reLP operations.

Proof of Concept

This is how mintokenAAmount is calculated inside reLP before performing the swap.

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

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

    // get rdpx price
>>> tokenAInfo.tokenAPrice = IRdpxEthOracle(addresses.rdpxOracle)
       .getRdpxPriceInEth();    

    // calculate min amount of tokenA to be received
>>> mintokenAAmount =
      (((amountB / 2) * tokenAInfo.tokenAPrice) / 1e8) -
      (((amountB / 2) * tokenAInfo.tokenAPrice * slippageTolerance) / 1e16);

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

    // ...
}

Inside the code, tokenAPrice is get from getRdpxPriceInEth. Which means tokenAPrice is ETH/RDPX.

Now, this is the mintokenAAmount calculation :

mintokenAAmount (RDPX) = (((amountB (ETH) / 2) x tokenAInfo.tokenAPrice (ETH/RDPX))) -
      (((amountB (ETH) / 2) x tokenAInfo.tokenAPrice (ETH/RDPX) x slippageTolerance))

It can be observed that amountB should be divided instead of multiplied by tokenAPrice to get the correct result.

Tools Used

Manual review

Recommended Mitigation Steps

Update the mintokenAAmount so it utilize tokenAPrice correctly :

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

    // get rdpx price
    tokenAInfo.tokenAPrice = IRdpxEthOracle(addresses.rdpxOracle)
       .getRdpxPriceInEth();    

    // calculate min amount of tokenA to be received
-    mintokenAAmount =
-      (((amountB / 2) * tokenAInfo.tokenAPrice) / 1e8) -
-      (((amountB / 2) * tokenAInfo.tokenAPrice * slippageTolerance) / 1e16);
+    mintokenAAmount =
+      (((amountB / 2) * 1e8) / tokenAInfo.tokenAPrice) -
+      (((amountB / 2) * 1e8 * slippageTolerance) / (1e8 * tokenAInfo.tokenAPrice));

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

    // ...
}

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

When upper depeg or lower depeg is performed, it will try to trigger _curveSwap to swap rdpxETH/ETH according to the current context of the operations. However, mintOut is calculated using wrong price when this operations are performed.

Proof of Concept

When upperDepeg and lowerDepeg functions are called, it will call _curveSwap and provide _ethToDpxEth as direction for the swap.

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

  function upperDepeg(
    uint256 _amount,
    uint256 minOut
  ) external onlyRole(DEFAULT_ADMIN_ROLE) returns (uint256 wethReceived) {
    _isEligibleSender();

    _validate(getDpxEthPrice() > 1e8, 10);

    IDpxEthToken(reserveAsset[reservesIndex["DPXETH"]].tokenAddress).mint(
      address(this),
      _amount
    );

    // Swap DpxEth for ETH token
>>> wethReceived = _curveSwap(_amount, false, true, minOut);

    reserveAsset[reservesIndex["WETH"]].tokenBalance += wethReceived;

    emit LogUpperDepeg(_amount, wethReceived);
  }

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

  function lowerDepeg(
    uint256 _rdpxAmount,
    uint256 _wethAmount,
    uint256 minamountOfWeth,
    uint256 minOut
  ) external onlyRole(DEFAULT_ADMIN_ROLE) returns (uint256 dpxEthReceived) {
    _isEligibleSender();
    _validate(getDpxEthPrice() < 1e8, 13);

    uint256 amountOfWethOut;

    if (_rdpxAmount != 0) {
      address[] memory path;
      path = new address[](2);
      path[0] = reserveAsset[reservesIndex["RDPX"]].tokenAddress;
      path[1] = weth;

      amountOfWethOut = IUniswapV2Router(addresses.dopexAMMRouter)
        .swapExactTokensForTokens(
          _rdpxAmount,
          minamountOfWeth,
          path,
          address(this),
          block.timestamp + 10
        )[path.length - 1];

      reserveAsset[reservesIndex["RDPX"]].tokenBalance -= _rdpxAmount;
    }

    // update Reserves
    reserveAsset[reservesIndex["WETH"]].tokenBalance -= _wethAmount;

>>> dpxEthReceived = _curveSwap(
      _wethAmount + amountOfWethOut,
      true,
      true,
      minOut
    );

    IDpxEthToken(reserveAsset[reservesIndex["DPXETH"]].tokenAddress).burn(
      dpxEthReceived
    );

    emit LogLowerDepeg(_rdpxAmount, _wethAmount, dpxEthReceived);
  }

However, when calculating minOut inside _curveSwap, wrong price will be used.

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

  function _curveSwap(
    uint256 _amount,
    bool _ethToDpxEth,
    bool validate,
    uint256 minAmount
  ) internal returns (uint256 amountOut) {
    IStableSwap dpxEthCurvePool = IStableSwap(addresses.dpxEthCurvePool);

    // First compute a reverse swapping of dpxETH to ETH to compute the amount of ETH required
    address coin0 = dpxEthCurvePool.coins(0);
    (uint256 a, uint256 b) = coin0 == weth ? (0, 1) : (1, 0);

    // validate the swap for peg functions
    if (validate) {
      uint256 ethBalance = IStableSwap(addresses.dpxEthCurvePool).balances(a);
      uint256 dpxEthBalance = IStableSwap(addresses.dpxEthCurvePool).balances(
        b
      );
      _ethToDpxEth
        ? _validate(
          ethBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
          14
        )
        : _validate(
          dpxEthBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
          14
        );
    }

    // calculate minimum amount out
>>> uint256 minOut = _ethToDpxEth
      ? (((_amount * getDpxEthPrice()) / 1e8) -
        (((_amount * getDpxEthPrice()) * slippageTolerance) / 1e16))
      : (((_amount * getEthPrice()) / 1e8) -
        (((_amount * getEthPrice()) * slippageTolerance) / 1e16));

    // Swap the tokens
    amountOut = dpxEthCurvePool.exchange(
      _ethToDpxEth ? int128(int256(a)) : int128(int256(b)),
      _ethToDpxEth ? int128(int256(b)) : int128(int256(a)),
      _amount,
      minAmount > 0 ? minAmount : minOut
    );
  }

If _ethToDpxEth is true, means we want to swap ETH to rdpxETH, the provided _amount will be in ETH, and minOut will be in rdpxETH. But the used price is getDpxEthPrice (ETH/rdpxETH). so the operation will be result in wrong value.

// this will result in wrong minAmount
minOut (rdpxETH) = _amount (ETH) x getDpxEthPrice (ETH/rdpxETH)

The same thing also happened when _ethToDpxEth is false, we want to swap rdpxETH to ETH, provided _amount in rdpxETH and minOut will be in ETH. But used getEthPrice (rdpxETH/ETH).

This upper depeg and lower depeg is a crucial function to adjust the price of rdpxETH/ETH, calculating wrong minOut could lead to upper/lower depeg calls will not work as expected and potentially not reach its goal.

Tools Used

Manual review

Manual review

Recommended Mitigation Steps

Adjust the price used accordingly :

  function _curveSwap(
    uint256 _amount,
    bool _ethToDpxEth,
    bool validate,
    uint256 minAmount
  ) internal returns (uint256 amountOut) {
    IStableSwap dpxEthCurvePool = IStableSwap(addresses.dpxEthCurvePool);

    // First compute a reverse swapping of dpxETH to ETH to compute the amount of ETH required
    address coin0 = dpxEthCurvePool.coins(0);
    (uint256 a, uint256 b) = coin0 == weth ? (0, 1) : (1, 0);

    // validate the swap for peg functions
    if (validate) {
      uint256 ethBalance = IStableSwap(addresses.dpxEthCurvePool).balances(a);
      uint256 dpxEthBalance = IStableSwap(addresses.dpxEthCurvePool).balances(
        b
      );
      _ethToDpxEth
        ? _validate(
          ethBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
          14
        )
        : _validate(
          dpxEthBalance + _amount <= (ethBalance + dpxEthBalance) / 2,
          14
        );
    }

    // calculate minimum amount out
-    uint256 minOut = _ethToDpxEth
-      ? (((_amount * getDpxEthPrice()) / 1e8) -
-        (((_amount * getDpxEthPrice()) * slippageTolerance) / 1e16))
-      : (((_amount * getEthPrice()) / 1e8) -
-        (((_amount * getEthPrice()) * slippageTolerance) / 1e16));
+    uint256 minOut = _ethToDpxEth
+      ? (((_amount * getEthPrice()) / 1e8) -
+        (((_amount * getEthPrice()) * slippageTolerance) / 1e16))
+      : (((_amount * getDpxEthPrice()) / 1e8) -
+        (((_amount * getDpxEthPrice()) * slippageTolerance) / 1e16));

    // Swap the tokens
    amountOut = dpxEthCurvePool.exchange(
      _ethToDpxEth ? int128(int256(a)) : int128(int256(b)),
      _ethToDpxEth ? int128(int256(b)) : int128(int256(a)),
      _amount,
      minAmount > 0 ? minAmount : minOut
    );
  }

Assessed type

Oracle

Lines of code

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

Vulnerability details

Impact

Current design allow user call bondWithDelegate using decaying bonds, this will make delegatee incur loss because they will get less bond and paid more eth.

Proof of Concept

When users have decaying bonds, they can either utilize it via bond or bondWithDelegate. If user choose to use bondWithDelegate, they need to provide _delegateIds (delegated eth that can fulfill the request).

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

  function bondWithDelegate(
    address _to,
    uint256[] memory _amounts,
    uint256[] memory _delegateIds,
    uint256 rdpxBondId
  ) public returns (uint256 receiptTokenAmount, uint256[] memory) {
    _whenNotPaused();
    // Validate amount
    _validate(_amounts.length == _delegateIds.length, 3);

    uint256 userTotalBondAmount;
    uint256 totalBondAmount;

    uint256[] memory delegateReceiptTokenAmounts = new uint256[](
      _amounts.length
    );

    for (uint256 i = 0; i < _amounts.length; i++) {
      // Validate amount
      _validate(_amounts[i] > 0, 4);

      BondWithDelegateReturnValue
        memory returnValues = BondWithDelegateReturnValue(0, 0, 0, 0);

>>>   returnValues = _bondWithDelegate(
        _amounts[i],
        rdpxBondId,
        _delegateIds[i]
      );

      delegateReceiptTokenAmounts[i] = returnValues.delegateReceiptTokenAmount;

      userTotalBondAmount += returnValues.bondAmountForUser;
      totalBondAmount += _amounts[i];

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

      // transfer the required rdpx
      _transfer(
        returnValues.rdpxRequired,
        returnValues.wethRequired - premium,
        _amounts[i],
        rdpxBondId
      );
    }

   // ....
  }

Then it will call _bondWithDelegate to calculate rdpxRequired and wethRequired and reduce delegatee provided collateral by increasing activeCollateral with wethRequired. Then calculate the bond received by calling _calculateAmounts, this will based on delegate.fee, wethRequired and rdpxRequired.

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

  function _bondWithDelegate(
    uint256 _amount,
    uint256 rdpxBondId,
    uint256 delegateId
  ) internal returns (BondWithDelegateReturnValue memory returnValues) {
    // Compute the bond cost
>>> (uint256 rdpxRequired, uint256 wethRequired) = calculateBondCost(
      _amount,
      rdpxBondId
    );

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

    Delegate storage delegate = delegates[delegateId];

    // update delegate active collateral
    _validate(delegate.amount - delegate.activeCollateral >= wethRequired, 5);
    delegate.activeCollateral += wethRequired;

    // update total weth delegated
    totalWethDelegated -= wethRequired;

    // Calculate the amount of bond token to mint for the delegate and user based on the fee
>>> (uint256 amount1, uint256 amount2) = _calculateAmounts(
      wethRequired,
      rdpxRequired,
      _amount,
      delegate.fee
    );

    // update user amounts
    // ETH token amount remaining after LP for the user
    uint256 bondAmountForUser = amount1;

    // Mint bond token for delegate
    // ETH token amount remaining after LP for the delegate
    uint256 delegateReceiptTokenAmount = _stake(delegate.owner, amount2);

    returnValues = BondWithDelegateReturnValue(
      delegateReceiptTokenAmount,
      bondAmountForUser,
      rdpxRequired,
      wethRequired
    );
  }

If user using decaying bonds, when call calculateBondCost and calculating rdpxRequired and wethRequired, it will not use any discount, this will make wethRequired even more bigger considering that if putOptionsRequired is true. delegatee need to paid premium that based on rdpxRequired (that not discounted).

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

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

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

      wethRequired =
        ((ETH_RATIO_PERCENTAGE - (bondDiscount / 2)) * _amount) /
        (DEFAULT_PRECISION * 1e2);
    } else {
      // if rdpx decaying bonds are being used there is no discount
      rdpxRequired =
        (RDPX_RATIO_PERCENTAGE * _amount * DEFAULT_PRECISION) /
        (DEFAULT_PRECISION * rdpxPrice * 1e2);

>>>   wethRequired =
        (ETH_RATIO_PERCENTAGE * _amount) /
        (DEFAULT_PRECISION * 1e2);
    }

    uint256 strike = IPerpetualAtlanticVault(addresses.perpetualAtlanticVault)
      .roundUp(rdpxPrice - (rdpxPrice / 4)); // 25% below the current price

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

This means, delegatee need to paid undiscounted wethRequired plus undiscounted premium!

This will make delegatee would not want to fulfill bondWithDelegate when decaying bond is used. Delegatee can front-run by withdrawing his delegated assets before bondWithDelegate with using decaying bonds calls.

PoC Foundry :

Comparing fulfilling bondWithDelegate requests in 2 scenarios, with decaying bonds and non-decaying bonds.

Add this test to Unit contract inside /tests/rdpxV2-core/Unit.t.sol, also add import "forge-std/console.sol"; in the contract :

   function testBondWithDelegateCompare() public {
        address alice = makeAddr("alice");
        rdpx.mint(alice, 1000 * 1e18);

        // test with rdpx decaying bonds
        rdpxDecayingBonds.grantRole(
            rdpxDecayingBonds.MINTER_ROLE(),
            address(this)
        );
        rdpxDecayingBonds.mint(alice, block.timestamp + 10, 125 * 1e16);
        assertEq(rdpxDecayingBonds.ownerOf(1), alice);
        /// add to delegate with different fees
        uint256 delegateId = rdpxV2Core.addToDelegate(50 * 1e18, 10 * 1e8);
        // uint256 delgateId2 = rdpxV2Core.addToDelegate(10 * 1e18, 20 * 1e8);

        // test bond with delegate
        uint256[] memory _amounts = new uint256[](1);
        uint256[] memory _delegateIds = new uint256[](1);
        _delegateIds[0] = 0;
        _amounts[0] = 1 * 1e18;

        // address 1 bonds
        /// check user balance
        uint256 userRdpxBalance = rdpx.balanceOf(alice);
        uint256 userWethBalance = weth.balanceOf(alice);

        (uint256 rdpxRequired, uint256 wethRequired) = rdpxV2Core
            .calculateBondCost(1 * 1e18, 0);

        vm.startPrank(alice);
        rdpx.approve(address(rdpxV2Core), type(uint256).max);

        (uint256 userAmount, uint256[] memory delegateAmounts) = rdpxV2Core
            .bondWithDelegate(alice, _amounts, _delegateIds, 0);
        vm.stopPrank();
        console.log("weth paid without decaying bond");
        console.log(wethRequired);
        console.log("delegatee received amount without decaying bond");
        console.log(delegateAmounts[0]);

        vm.startPrank(alice);
        rdpx.approve(address(rdpxV2Core), type(uint256).max);
        // with decaying bond
        (uint256 rdpxRequired2, uint256 wethRequired2) = rdpxV2Core
            .calculateBondCost(1 * 1e18, 1);
        (uint256 userAmount2, uint256[] memory delegateAmounts2) = rdpxV2Core
            .bondWithDelegate(alice, _amounts, _delegateIds, 1);
        vm.stopPrank();

        console.log("weth paid with decaying bond");
        console.log(wethRequired2);
        console.log("delegatee received amount with decaying bond");
        console.log(delegateAmounts2[0]);
    }

Run the test :

forge test --match-contract Unit --match-test testBondWithDelegateCompare -vvv

Test Output :

Logs:
  weth paid without decaying bond
  806250000000000000
  delegatee received amount without decaying bond
  197562425683709869

  weth paid with decaying bond
  812500000000000000
  delegatee received amount with decaying bond
  197058823529411765

It can be observed delegatee paid weth more with decaying bond and receive less bond share.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider to restrict bondWithDelegate for only non-decaying bonds. Or also add discount but apply only to the weth part if called from bondWithDelegate.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

When user execute bond operations and putOptionsRequired is true, the premium that they will pay is depend on time when the bond operations will be executed. This will cause the premium paid by user will be less despite bond the same amount at the same price if they time it correctly.

Proof of Concept

When user trigger bond or bondWithDelegate, it will call calculateBondCost to calculate rdpxRequired and wethRequired.

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

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

    _transfer(rdpxRequired, wethRequired - premium, _amount, rdpxBondId);

    // Stake the ETH in the ReceiptToken contract
    receiptTokenAmount = _stake(_to, _amount);

    // reLP
    if (isReLPActive) IReLP(addresses.reLPContract).reLP(_amount);

    emit LogBond(rdpxRequired, wethRequired, receiptTokenAmount);
  }

If putOptionsRequired is true, calculateBondCost will calculate the premium users need to pay and add it to wethRequired :

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

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

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

      wethRequired =
        ((ETH_RATIO_PERCENTAGE - (bondDiscount / 2)) * _amount) /
        (DEFAULT_PRECISION * 1e2);
    } else {
      // if rdpx decaying bonds are being used there is no discount
      rdpxRequired =
        (RDPX_RATIO_PERCENTAGE * _amount * DEFAULT_PRECISION) /
        (DEFAULT_PRECISION * rdpxPrice * 1e2);

      wethRequired =
        (ETH_RATIO_PERCENTAGE * _amount) /
        (DEFAULT_PRECISION * 1e2);
    }

    uint256 strike = IPerpetualAtlanticVault(addresses.perpetualAtlanticVault)
      .roundUp(rdpxPrice - (rdpxPrice / 4)); // 25% below the current price

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

It can be observed that the provided timeToExpiry is depend on nextFundingPaymentTimestamp and current block.timestamp.

This has potential leak value issue, since with providing the same amount at the same price (assume the rdpx price will not be that volatile), the user paid less premium by timing the bond near to nextFundingPaymentTimestamp.

Foundry PoC :

The goal of this PoC is we try to compare the option price we get, using OptionPricingSimple, providing the same amount and price, with different time. OptionPricingSimple configured with 0.05% minimum price, funding duration 7 days and current rdpxPriceInEth is 2e7.

Compared time 7 days, 6 days, and 1 day before next funding payment.

Add this test to Unit contract inside /tests/rdpxV2-core/Unit.t.sol, also add import "forge-std/console.sol"; and import {OptionPricingSimple} from "contracts/libraries/OptionPricingSimple.sol"; in the contract :

    function testOptionPricingTiming() public {
        OptionPricingSimple optionPricingSimple;
        // 5e6
        optionPricingSimple = new OptionPricingSimple(100, 5e6);

        (uint256 rdpxRequired, uint256 wethRequired) = rdpxV2Core
            .calculateBondCost(1 * 1e18, 0);

        uint256 currentPrice = vault.getUnderlyingPrice(); // price
        uint256 strike = vault.roundUp(currentPrice - (currentPrice / 4)); // 25% below the current price
        console.log("What is the current price");
        console.log(currentPrice);
        console.log("What is the strike");
        console.log(strike);

        // 7 days before next funding payment
        uint256 timeToExpiry = vault.nextFundingPaymentTimestamp() -
            block.timestamp;
        console.log("What is time to expiry initial :");
        console.log(timeToExpiry);
        uint256 optionPrice = optionPricingSimple.getOptionPrice(
            strike,
            currentPrice,
            100,
            timeToExpiry
        );
        console.log("What is option pricing initial :");
        console.log(optionPrice);

        // 1 day afters
        vm.warp(block.timestamp + 1 days);
        uint256 timeToExpiryAfter1Day = vault.nextFundingPaymentTimestamp() -
            block.timestamp;
        console.log("What is time to expiry after 1 day :");
        console.log(timeToExpiryAfter1Day);
        optionPrice = optionPricingSimple.getOptionPrice(
            strike,
            currentPrice,
            100,
            timeToExpiryAfter1Day
        );
        console.log("What is option pricing after 1 day :");
        console.log(optionPrice);

        // after 6 days
        vm.warp(block.timestamp + 5 days);
        uint256 timeToExpiryAfter6Day = vault.nextFundingPaymentTimestamp() -
            block.timestamp;
        console.log("What is time to expiry after 6 day :");
        console.log(timeToExpiryAfter6Day);

        optionPrice = optionPricingSimple.getOptionPrice(
            strike,
            currentPrice,
            100,
            timeToExpiryAfter6Day
        );
        console.log("What is option pricing after 6 day :");
        console.log(optionPrice);
    }

Run the PoC :

forge test --match-contract Unit --match-test testOptionPricingTiming -vvv

Log Output :

Logs:
  What is the current price
  20000000
  What is the strike
  15000000

  What is time to expiry initial :
  604800
  What is option pricing initial :
  16481

  What is time to expiry after 1 day :
  518400
  What is option pricing after 1 day :
  10000

  What is time to expiry after 6 day :
  86400
  What is option pricing after 6 day :
  10000

It can be observed that users will pay minimum price (0.05%) even only after 1 day updating funding time pointer!

Tools Used

Manual review

Recommended Mitigation Steps

To avoid this, fix the provided Black-Scholes option price observation time to funding epoch duration windows :

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

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

      wethRequired =
        ((ETH_RATIO_PERCENTAGE - (bondDiscount / 2)) * _amount) /
        (DEFAULT_PRECISION * 1e2);
    } else {
      // if rdpx decaying bonds are being used there is no discount
      rdpxRequired =
        (RDPX_RATIO_PERCENTAGE * _amount * DEFAULT_PRECISION) /
        (DEFAULT_PRECISION * rdpxPrice * 1e2);

      wethRequired =
        (ETH_RATIO_PERCENTAGE * _amount) /
        (DEFAULT_PRECISION * 1e2);
    }

    uint256 strike = IPerpetualAtlanticVault(addresses.perpetualAtlanticVault)
      .roundUp(rdpxPrice - (rdpxPrice / 4)); // 25% below the current price

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

Also update the time expiry when purchase the options

  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;
+    uint256 timeToExpiry = nextFundingPaymentTimestamp() - fundingDuration
    // Get total premium for all options being purchased
    premium = calculatePremium(strike, amount, timeToExpiry, 0);

    // Transfer premium from msg.sender to PerpetualAtlantics vault
    collateralToken.safeTransferFrom(msg.sender, address(this), premium);

    perpetualAtlanticVaultLp.lockCollateral(requiredCollateral);
    _updateFundingRate(premium);

    // Mint the option tokens
    tokenId = _mintOptionToken();
    optionPositions[tokenId] = OptionPosition({
      strike: strike,
      amount: amount,
      positionId: tokenId
    });

    totalActiveOptions += amount;
    fundingPaymentsAccountedFor[latestFundingPaymentPointer] += amount;
    optionsPerStrike[strike] += amount;

    // record the number of options funding has been accounted for the epoch and strike
    fundingPaymentsAccountedForPerStrike[latestFundingPaymentPointer][
      strike
    ] += amount;

    emit Purchase(strike, amount, premium, to, msg.sender);
  }

Assessed type

Context