code-423n4 / 2024-06-vultisig-validation

2 stars 0 forks source link

Missing deadline for slippage in `addLiquidity` #248

Open c4-bot-9 opened 5 months ago

c4-bot-9 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/main/src/ILOPool.sol#L296

Vulnerability details

Impact

The absence of a deadline check in the addLiquidity function can lead to transactions being executed at a much later time than intended, exposing users to significant price volatility. This can result in unfavorable exchange rates and potential financial losses.

Proof of Concept

Below is an example of the addLiquidity function in the LiquidityManagement which ls used in the ILOPool contract without a deadline check:

struct AddLiquidityParams {
    IUniswapV3Pool pool;
    uint128 liquidity;
    uint256 amount0Desired;
    uint256 amount1Desired;
    uint256 amount0Min;
    uint256 amount1Min;
}

function addLiquidity(AddLiquidityParams memory params)
    internal
    returns (
        uint256 amount0,
        uint256 amount1
    )
{
    (amount0, amount1) = params.pool.mint(
        address(this),
        TICK_LOWER,
        TICK_UPPER,
        params.liquidity,
        ""
    );

    require(amount0 >= params.amount0Min && amount1 >= params.amount1Min, 'Price slippage check');
}

this is later used in the ILOPool contract as below when launching

// actually deploy liquidity to uniswap pool
            (amount0, amount1) = addLiquidity(AddLiquidityParams({
                pool: IUniswapV3Pool(uniV3PoolAddress),
                liquidity: liquidity,
                amount0Desired: amount0,
                amount1Desired: amount1,
                amount0Min: amount0Min,
                amount1Min: amount1Min
            }));

In this implementation, there is no check to ensure that the transaction is executed within a certain timeframe.

Tools Used

Manual Review

Recommended Mitigation Steps

update the AddLiquidityParams struct to include a deadline parameter and modify the addLiquidity function to include a check against this deadline.

Assessed type

MEV

pratokko commented 4 months ago

Hello judge, even though this function is handling slippage it is still missing a deadline check, it is very common for the addLiquidity function to include a deadline please help me review this thank you!

alex-ppg commented 4 months ago

Hey @pratokko, thank you for the PJQA contribution. I will preface all validation repository finding responses by stating that they are not evaluated by judges directly and are only evaluated by the validators if they are deemed unsatisfactory.

The liquidity provision highlighted is a full-range liquidity provision that is performed once during a project's launch. We can safely assume that no deadline is meant to be imposed in this particular case as no other user can supply liquidity to the pair until this point.

This paragraph is included in all of my responses and confirms that no further feedback is expected in this submission as PJQA has concluded. You are free to refute any of my statements factually, however, I strongly implore you to do this with actual code references and examples.