mintTokenizedPosition doesn't use slippage protection although slippageTickLimitLow, slippageTickLimitHigh are provided in parameters

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L504-L508 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L718 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L837-L845

Vulnerability details

Impact

Slippage protection won't be provided to users even though they set parameters for doing so.

Proof of Concept

mintTokenizedPosition takes slippageTickLimitLow and slippageTickLimitHigh as parameters to provide slippage protection to user while swapping for ITM options in Uniswap V3, however they're not used as the default MAX and MIN possible sqrtPriceX96 is used instead. Providing zero slippage protection.

mintTokenizedPosition -> _validateAndForwardToAMM -> swapInAMM

mintTokenizedPosition

    function mintTokenizedPosition(
        TokenId tokenId,
        uint128 positionSize,
        int24 slippageTickLimitLow,
        int24 slippageTickLimitHigh
    )
{
    .
    .
        // validate the incoming option position, then forward to the AMM for minting/burning required liquidity chunks
        (collectedByLeg, totalSwapped) = _validateAndForwardToAMM(
            tokenId,
            positionSize,
            slippageTickLimitLow,
            slippageTickLimitHigh,
            MINT
        );
}

_validateAndForwardToAMM

    /// @param tokenId the option position
    /// @param positionSize the size of the position to create
@>  /// @param tickLimitLow lower limits on potential slippage // not used much
@>  /// @param tickLimitHigh upper limits on potential slippage
    /// @param isBurn is equal to false for mints and true for burns
    /// @return collectedByLeg An array of LeftRight encoded words containing the amount of token0 and token1 collected as fees for each leg
    /// @return totalMoved the total amount of funds swapped in Uniswap as part of building potential ITM positions
    function _validateAndForwardToAMM(
        TokenId tokenId,
        uint128 positionSize,
        int24 tickLimitLow,
        int24 tickLimitHigh,
        bool isBurn
    ) 
{
    .
    .
        if (tickLimitLow > tickLimitHigh) {
            // if the in-the-money amount is not zero (i.e. positions were minted ITM) and the user did provide tick limits LOW > HIGH, then swap necessary amounts
            if ((LeftRightSigned.unwrap(itmAmounts) != 0)) {
                totalMoved = swapInAMM(univ3pool, itmAmounts).add(totalMoved);
            }

            (tickLimitLow, tickLimitHigh) = (tickLimitHigh, tickLimitLow);
        }
   .
   .
}

Swap in AMM

function swapInAMM(
        IUniswapV3Pool univ3pool,
        LeftRightSigned itmAmounts
    ) internal
{
            .
            .
            (int256 swap0, int256 swap1) = _univ3pool.swap(
                msg.sender,
                zeroForOne,
                swapAmount,
                zeroForOne
@>                  ? Constants.MIN_V3POOL_SQRT_RATIO + 1  
@>                  : Constants.MAX_V3POOL_SQRT_RATIO - 1,
// should be calculated from user slippage parameters
                data
            );
            .
            .
}

Uniswap swap parameter specification

    /// @notice Swap token0 for token1, or token1 for token0
    /// @dev The caller of this method receives a callback in the form of IUniswapV3SwapCallback#uniswapV3SwapCallback
    /// @param recipient The address to receive the output of the swap
    /// @param zeroForOne The direction of the swap, true for token0 to token1, false for token1 to token0
    /// @param amountSpecified The amount of the swap, which implicitly configures the swap as exact input (positive), or exact output (negative)
@>  /// @param sqrtPriceLimitX96 The Q64.96 sqrt price limit. If zero for one, the price cannot be less than this
    /// value after the swap. If one for zero, the price cannot be greater than this value after the swap
    /// @param data Any data to be passed through to the callback
    /// @return amount0 The delta of the balance of token0 of the pool, exact when negative, minimum when positive
    /// @return amount1 The delta of the balance of token1 of the pool, exact when negative, minimum when positive
    function swap(
        address recipient,
        bool zeroForOne,
        int256 amountSpecified,
        uint160 sqrtPriceLimitX96,
        bytes calldata data
    ) external returns (int256 amount0, int256 amount1);

Tools Used

Manual analysis

Recommended Mitigation Steps

The proper slippage limit should be set in the swapInAMM function, from parameters passed from _validateAndForwardToAMM

Assessed type

Invalid Validation

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

The check is done here https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/SemiFungiblePositionManager.sol#L727