Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/SemiFungiblePositionManager.sol#L679

Vulnerability details

Impact

In _validateAndForwardToAMM function, the validation logic checks if tickLimitLow is greater than tickLimitHigh after the _createPositionInAMM function call. If this condition is true, it swaps the values and performs the ITM swap if necessary. However, the slippage check occurs after these values are potentially swapped, which can result in incorrect validation if the original tickLimitLow was indeed greater than tickLimitHigh.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

LOC

function _validateAndForwardToAMM(
    TokenId tokenId,
    uint128 positionSize,
    int24 tickLimitLow,
    int24 tickLimitHigh,
    bool isBurn
) internal returns (LeftRightUnsigned[4] memory collectedByLeg, LeftRightSigned totalMoved) {
    // Reverts if positionSize is 0 and user did not own the position before minting/burning
    if (positionSize == 0) revert Errors.OptionsBalanceZero();

    // Validate tokenId
    tokenId.validate();

    /// @dev the flipToBurnToken() function flips the isLong bits
    if (isBurn) {
        tokenId = tokenId.flipToBurnToken();
    }

    // Extract univ3pool from the poolId map to Uniswap Pool
    IUniswapV3Pool univ3pool = s_poolContext[tokenId.poolId()].pool;

    // Revert if the pool not been previously initialized
    if (univ3pool == IUniswapV3Pool(address(0))) revert Errors.UniswapPoolNotInitialized();

    // initialize some variables returned by the _createPositionInAMM function
    LeftRightSigned itmAmounts;

    // calls a function that loops through each leg of tokenId and mints/burns liquidity in Uni v3 pool
    (totalMoved, collectedByLeg, itmAmounts) = _createPositionInAMM(
        univ3pool,
        tokenId,
        positionSize,
        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);
    }

    // Get the current tick of the Uniswap pool, check slippage
    (, int24 currentTick, , , , , ) = univ3pool.slot0();

    if ((currentTick >= tickLimitHigh) || (currentTick <= tickLimitLow))
        revert Errors.PriceBoundFail();
}

Tools Used

Manual code review.

Recommended Mitigation Steps

Validate the tickLimitLow and tickLimitHigh values before calling _createPositionInAMM. If tickLimitLow is greater than tickLimitHigh, revert the transaction immediately to prevent incorrect slippage checks.

function _validateAndForwardToAMM(
    TokenId tokenId,
    uint128 positionSize,
    int24 tickLimitLow,
    int24 tickLimitHigh,
    bool isBurn
) internal returns (LeftRightUnsigned[4] memory collectedByLeg, LeftRightSigned totalMoved) {
    // Reverts if positionSize is 0 and user did not own the position before minting/burning
    if (positionSize == 0) revert Errors.OptionsBalanceZero();

    // Validate tokenId
    tokenId.validate();

    /// @dev the flipToBurnToken() function flips the isLong bits
    if (isBurn) {
        tokenId = tokenId.flipToBurnToken();
    }

    // Extract univ3pool from the poolId map to Uniswap Pool
    IUniswapV3Pool univ3pool = s_poolContext[tokenId.poolId()].pool;

    // Revert if the pool not been previously initialized
    if (univ3pool == IUniswapV3Pool(address(0))) revert Errors.UniswapPoolNotInitialized();

    // Perform initial slippage check
    if (tickLimitLow > tickLimitHigh) {
        revert Errors.InvalidTickLimits();
    }

    // initialize some variables returned by the _createPositionInAMM function
    LeftRightSigned itmAmounts;

    // calls a function that loops through each leg of tokenId and mints/burns liquidity in Uni v3 pool
    (totalMoved, collectedByLeg, itmAmounts) = _createPositionInAMM(
        univ3pool,
        tokenId,
        positionSize,
        isBurn
    );

    // 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);
    }

    // Get the current tick of the Uniswap pool, check slippage
    (, int24 currentTick, , , , , ) = univ3pool.slot0();

    if ((currentTick >= tickLimitHigh) || (currentTick <= tickLimitLow))
        revert Errors.PriceBoundFail();
}

Assessed type

Invalid Validation

code-423n4 / 2024-06-panoptic-findings

Incorrect Validation for tickLimitLow and tickLimitHigh Causing Potential Slippage Check Failures #6