code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

`UniV3LiquidityAmo` has ineffective deadline check that could allow MEV #1262

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

UniV3LiquidityAmo provides addLiquidity() and removeLiquidity() to allow liquidity provisioning for RdpxV2Core on UniswapV3.

However, it uses the value type(uint256).max and block.timestamp for the deadline parameter for adding/removing liquidity. That is not recommended as the transactions could be possibly be re-ordered or delayed and executed at a later time with the malicious intent to obtain MEV from it.

Even though, Arbitrum does not have a mempool at the moment, there could possibly be MEV opportunities in the future when they decentralize the sequencer. And this issue is also applicable if the project intends to expand to other chains or make it omnichain.

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV3LiquidityAmo.sol#L190

    INonfungiblePositionManager.MintParams
      memory mintParams = INonfungiblePositionManager.MintParams(
        params._tokenA,
        params._tokenB,
        params._fee,
        params._tickLower,
        params._tickUpper,
        params._amount0Desired,
        params._amount1Desired,
        params._amount0Min,
        params._amount1Min,
        address(this),
        type(uint256).max
      );

    (uint256 tokenId, uint128 amountLiquidity, , ) = univ3_positions.mint(
      mintParams
    );

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/amo/UniV3LiquidityAmo.sol#L250

    // remove liquidity
    INonfungiblePositionManager.DecreaseLiquidityParams
      memory decreaseLiquidityParams = INonfungiblePositionManager
        .DecreaseLiquidityParams(
          pos.token_id,
          liquidity,
          minAmount0,
          minAmount1,
          block.timestamp
        );

    univ3_positions.decreaseLiquidity(decreaseLiquidityParams);

Impact

The add/remove liquidity transaction could be subjected to MEV due to the issue.

Recommended Mitigation Steps

Allow deadline to be passed in by the user as a parameter.

Assessed type

MEV

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #2217

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #898

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)