`PanopticFactory` uses spot price when deploying new pools, resulting in liquidity manipulation when minting

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L341-L374

Vulnerability details

Impact

When deployNewPool is called it uses the spot price of the pool, which can be manipulated through a flashloan and thus could return a highly inaccurate result.

The price is used when deciding how much liquidity should be minted for each token, so this can result in an unbalanced pool.

In other parts of the code, this is not an issue as there are oracles that prevent price manipulations, but in case there aren't any checks to avoid so.

Proof of Concept

The spot price is used to calculate the range liquidity for each token:

@>  (uint160 currentSqrtPriceX96, , , , , , ) = v3Pool.slot0();

    // For full range: L = Δx * sqrt(P) = Δy / sqrt(P)
    // We start with fixed token amounts and apply this equation to calculate the liquidity
    // Note that for pools with a tickSpacing that is not a power of 2 or greater than 8 (887272 % ts != 0),
    // a position at the maximum and minimum allowable ticks will be wide, but not necessarily full-range.
    // In this case, the `fullRangeLiquidity` will always be an underestimate in respect to the token amounts required to mint.
    uint128 fullRangeLiquidity;
    unchecked {
        // Since we know one of the tokens is WETH, we simply add 0.1 ETH + worth in tokens
        if (token0 == WETH) {
            fullRangeLiquidity = uint128(
@>              Math.mulDiv96RoundingUp(FULL_RANGE_LIQUIDITY_AMOUNT_WETH, currentSqrtPriceX96)
            );
        } else if (token1 == WETH) {
            fullRangeLiquidity = uint128(
                Math.mulDivRoundingUp(
                    FULL_RANGE_LIQUIDITY_AMOUNT_WETH,
                    Constants.FP96,
@>                  currentSqrtPriceX96
                )
            );
        } else {
            // Find the resulting liquidity for providing 1e6 of both tokens
            uint128 liquidity0 = uint128(
@>              Math.mulDiv96RoundingUp(FULL_RANGE_LIQUIDITY_AMOUNT_TOKEN, currentSqrtPriceX96)
            );
            uint128 liquidity1 = uint128(
                Math.mulDivRoundingUp(
                    FULL_RANGE_LIQUIDITY_AMOUNT_TOKEN,
                    Constants.FP96,
@>                  currentSqrtPriceX96
                )
            );

            // Pick the greater of the liquidities - i.e the more "expensive" option
            // This ensures that the liquidity added is sufficiently large
            fullRangeLiquidity = liquidity0 > liquidity1 ? liquidity0 : liquidity1;
        }
    }

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L341-L374

But unlike other parts of the code, the PanopticFactory doesn't have any checks against the price (it doesn't use any oracles nor the TWAP), so each token liquidity is manipulable through flash loans.

Tools Used

Manual review

Recommended Mitigation Steps

Consider using the TWAP price instead of the spot price.

Assessed type

Uniswap

[c4-judge]

Picodes marked the issue as primary issue

[dyedm1]

True, but I don't see negative consequences for this? This function is just a way to add some full-range liquidity to the pool so the entire range can be swapped across, and depending on the tokens we can add very small/large amounts of liquidity anyway (mentioned in the readme: Depending on the token, the amount of funds required for the initial factory deployment may be high or unrealistic)

[Picodes]

@dyedm1 assuming the deployer has infinite approvals, can't we imagine a scenario where, by manipulating the spot pool price, it ends up depositing way too many token1 and getting sandwiched leading to a significant loss?

Said differently the risk is that currently the deployer has no control over the amount of token1 he will donate and this amount can be manipulated by an attacker.

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[Picodes]

This is at most Medium to me considering pool deployers are advanced users and you need to deploy a pool where the manipulation cost is low which should remain exceptional.

[c4-judge]

Picodes marked the issue as selected for report

[dyedm1]

Yeah I agree this might be less than ideal if you have infinite approvals. Our UI doesn't do infinite approvals to the factory, but some wallets allow users to edit the approval amount before signing the transaction, so it might be prudent to add slippage checks here (to make the process idiot-proof).