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

0 stars 0 forks source link

Incorrect price limits during the initILOPool process may lead to incorrect pricing. #149

Open c4-bot-8 opened 2 months ago

c4-bot-8 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/cb72b1e9053c02a58d874ff376359a83dc3f0742/src/ILOManager.sol#L90

Vulnerability details

Impact

There is no check when initILOPool that initialPoolPriceX96 must be less than sqrtRatioUpperX96. As a result, initialPoolPriceX96 may be larger than initialPoolPriceX96. This may cause all ilopools corresponding to this uniV3pool to be unable to launch or cause users to receive fewer tokens and have liquidity stuck in the protocol.

Proof of Concept

Because initialPoolPriceX96 may be greater than initialPoolPriceX96. This may cause liquidityDelta to be incorrectly calculated when buying is calculated in the pool. Finally, during launch, totalRaised and liquidity could not correspond, resulting in failure to add liquidity or stranded funds.

        // get amount of liquidity associated with raise amount
        if (RAISE_TOKEN == _cachedPoolKey.token0) {
            liquidityDelta = LiquidityAmounts.getLiquidityForAmount0(SQRT_RATIO_X96, SQRT_RATIO_UPPER_X96, raiseAmount);
        } else {
            liquidityDelta = LiquidityAmounts.getLiquidityForAmount1(SQRT_RATIO_LOWER_X96, SQRT_RATIO_X96, raiseAmount);
        }
            // calculate sale amount of tokens needed for launching pool
            if (token0Addr == RAISE_TOKEN) {
                amount0 = totalRaised;
                amount0Min = totalRaised;
                (amount1, liquidity) = _saleAmountNeeded(totalRaised);
            } else {
                (amount0, liquidity) = _saleAmountNeeded(totalRaised);
                amount1 = totalRaised;
                amount1Min = totalRaised;
            }
        if (_cachedPoolKey.token0 == SALE_TOKEN) {
            // liquidity1 raised
            liquidity = LiquidityAmounts.getLiquidityForAmount1(SQRT_RATIO_LOWER_X96, SQRT_RATIO_X96, raiseAmount);
            saleAmountNeeded = SqrtPriceMathPartial.getAmount0Delta(SQRT_RATIO_X96, SQRT_RATIO_UPPER_X96, liquidity, true);
        } else {
            // liquidity0 raised
            liquidity = LiquidityAmounts.getLiquidityForAmount0(SQRT_RATIO_X96, SQRT_RATIO_UPPER_X96, raiseAmount);
            saleAmountNeeded = SqrtPriceMathPartial.getAmount1Delta(SQRT_RATIO_LOWER_X96, SQRT_RATIO_X96, liquidity, true);
        }

In addition, as long as one ilopool fails to launch, all corresponding ilopools cannot be launched, which has a greater impact.

Tools Used

manual

Recommended Mitigation Steps

-        require(sqrtRatioLowerX96 < _project.initialPoolPriceX96 && sqrtRatioLowerX96 < sqrtRatioUpperX96, "RANGE");
+        require(sqrtRatioLowerX96 < _project.initialPoolPriceX96 && initialPoolPriceX96 < sqrtRatioUpperX96, "RANGE");

Assessed type

Other

Scorpiondeng commented 1 month ago

Hi @alex-ppg thank you for your judgement Please re-evaluate this finding. Sorry for the error in the report, it should be "initialPoolPriceX96 may be greater than sqrtRatioUpperX96." This can be stated in the fix suggestion.

Scorpiondeng commented 1 month ago

Although Non privileged users are expected to preview their transactions to protect against input mistakes. Anyone using modern tooling will have previews built into their toolset., errors do not appear immediately and therefore cannot be found in the preview.

Assume the following situation, the user wants to create multiple pools on a project. For example pool1, pool2.

  1. Assume that pool1 uses wrong parameters when initILOPool. sqrtRatioUpperX96 calculated by tickUpper is less than _project.initialPoolPriceX96. Since the user passes in tickUpper, this needs to calculate sqrtRatioUpperX96. Wrong parameters are not easy to spot and no errors will appear in the preview.
  2. The launch function will not be called until a period of time after raising funds. At this time, liquidity cannot be added due to incorrect price settings, and the launch will fail. This will cause the protocol functionality to become unusable. And user funds have been locked for a period of time during this period. So I think it's Med.
alex-ppg commented 1 month ago

Hey @Scorpiondeng, 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.

Duplicates of your submission exist in the findings repository and were deemed unsatisfactory because they detail a misconfiguration of the system. It is in the configurator's best interest to set a proper initial pool price, and failure to do so would only affect them as the project could still proceed with refunds. As such, this submission would at most be considered QA.

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.