Dopex - Norah's results

Lines of code

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

Vulnerability details

Description

rpdxV2core.sol buys the put option for rdpx token (underlying) in Weth from the PerprtualAtlanticVault.sol, if the option turns out ITM (In the Money), the DEFAULT_ADMIN_ROLE calls settle() function of 1rpdxV2core.sol` contract.

Which in turn calls PerprtualAtlanticVault.sol contracts settle() function where in,

• Agreed amount of rpdx tokens are transfer to the PerprtualAtlanticVaultLP.sol from the rpdxV2core.sol
• Weth amount as per the strike price is transfer from PerprtualAtlanticVaultLP.sol to rpdxV2core.sol.
• After this particular transfer there is call to subtractLoss() function of PerprtualAtlanticVaultLP.sol. -In subtractLoss function , a check is performed to ensure that the collateral (Weth) owned by PerprtualAtlanticVaultLP.sol matches the internal accounting of collateral minus the transferred amount (loss).

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

The issue arises due to the specific check mentioned above, where the protocol checks if the collateral (ETH) owned by PerprtualAtlanticVaultLP.sol is equal to the internal accounting of collateral minus the transferred amount. This check utilizes the collateral.balanceOf(address(this)) function to determine the current ownership of the collateral (ETH). This can be manipulated through external transfers, potentially causing a discrepancy between the actual balance and the internal accounting.

An attacker could transfer a tiny amount of collateral (e.g., 1 wei worth of ETH) to PerprtualAtlanticVaultLP.sol contract from an external source, which would make collateral.balanceOf(address(this)) always 1 wei higher than the internal accounting. This manipulation would cause the mentioned check to fail, leading to a transaction revert and effectively performing a Denial of Service (DOS) attack on the settle() functionality.

Also, since there is no way to remove or sweep unaccounted collateral or forcefully sync internal accounting to the same, protocol will be permanently in DOS, will have to redeployed the vault for normal functioning.

Impact

The vulnerability results in a sustained denial of service attack targeting the settle() functionality. This not only disrupts the protocol's core operation but also necessitates the redeployment of the entire protocol to restore normal functioning.

Proof of Concept

Tools Used

Manual Review

Recommended Mitigation Steps

To mitigate this vulnerability :

1. Remove the follwing problematic check that relies on collateral.balanceOf(address(this)) in PerprtualAtlanticVaultLP.sol contracts subtractLoss() function.

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

2. Implement a new check directly within the PerprtualAtlanticVault.sol contract's settle() function.

   • The new check should involve comparing the intended ETH transfer amount with the difference between the cached balance of collateral before and after the transfer. This approach ensures that the check is atomic and external transfers wont affect anything anymore.

    function settle(
    uint256[] memory optionIds
  )
    external
    nonReentrant
    onlyRole(RDPXV2CORE_ROLE)
    returns (uint256 ethAmount, uint256 rdpxAmount)
  {
    
    .
    .
    .
    .

    uint256 balanceBefore = collateralToken.balanceOf(Addresses.perpetualAtlanticVaultLP);

    collateralToken.safeTransferFrom(
      addresses.perpetualAtlanticVaultLP,
      addresses.rdpxV2Core,
      ethAmount
    );

    uint256 balanceAfter = collateralToken.balanceOf(ddresses.perpetualAtlanticVaultLP);
    
    require(ethAmount == balanceAfter - balanceBefore, "Incorrect transfer");

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

Assessed type

DoS