Dopex - blutorque's results

Lines of code

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

Vulnerability details

Impact

Every option that has been purchased has unique optionId. This id represent the current position of optionHolder includes strike price at which option bought and the amount purchased.

A person with DEFAULT_ADMIN_ROLE can directly settles these options via call to RdpxV2Core#settle(), which can be DoS by an attacker by directly transfer funds to the LP Vault.

File: RdpxV2Core.sol

  function settle(
    uint256[] memory optionIds
  )
    external onlyRole(DEFAULT_ADMIN_ROLE)
    returns (uint256 amountOfWeth, uint256 rdpxAmount)
  {
    // ...SNIP...
    (amountOfWeth, rdpxAmount) = IPerpetualAtlanticVault(
      addresses.perpetualAtlanticVault
    ).settle(optionIds);
    // ...SNIP...
  }

File: PerpetualAtlanticVault.sol

  function settle(
    uint256[] memory optionIds
  )
    external
    nonReentrant
    onlyRole(RDPXV2CORE_ROLE)
    returns (uint256 ethAmount, uint256 rdpxAmount)
  {
    _whenNotPaused();
    _isEligibleSender();
    // ...SNIP...
        // Transfer collateral token from perpetual vault to rdpx rdpxV2Core
    collateralToken.safeTransferFrom(
      addresses.perpetualAtlanticVaultLP,
      addresses.rdpxV2Core,
      ethAmount
    );
    // Transfer rdpx from rdpx rdpxV2Core to perpetual vault
    IERC20WithBurn(addresses.rdpx).safeTransferFrom(
      addresses.rdpxV2Core,
      addresses.perpetualAtlanticVaultLP,
      rdpxAmount
    );
    IPerpetualAtlanticVaultLP(addresses.perpetualAtlanticVaultLP).subtractLoss(ethAmount); // update the totalCollateral state
    // ...SNIP...
  }

The above snippet shows IPerpetualAtlanticVault(address).settle() function transfer the collateral(e.g. weth) from LpVault to the RdpxV2Core which decreases collateral balance of LpVault.

File: PerpetualAtlanticVaultLP.sol

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

Following the change in collateral balance, the call to subtractLoss() function reduces the _totalCollateral by the amount that being transferred to RdpxV2Core. Its looks like here the developer pre-assumed that the collateral balance should be equal to _totalColllateral before its call to subtractLoss(). The assumption can be break easily by transferring the minimum 1 wei of colllateral directly to the LpVault contract.

Hence, the check will always failed and revert any txn that came to the option settlement.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Tools Used

Manual review

Recommended Mitigation Steps

I discussed some fixes with sponser, but it looks like there will always be risk because we can't control the collateral balance in contract. Therefore its in our best interest to remove the check. Modify the subtractLoss() function to below.

  function subtractLoss(uint256 loss) public onlyPerpVault {
    _totalCollateral -= loss;
  }

Assessed type

DoS