Lack of Validation for positionIdList in mintOptions Function Can Lead to Errors and Potential Exploits

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/main/contracts/PanopticPool.sol#L591

Vulnerability details

Impact

The mintOptions function in the PanopticPool contract has a potential oversight related to the validation of the positionIdList parameter. If positionIdList is empty or contains invalid TokenIds, it could lead to unexpected behavior, errors, or even potential exploits. This oversight could allow users to bypass necessary checks, leading to incorrect minting of options and potentially compromising the integrity of the system.

Proof of Concept

function mintOptions(
    TokenId[] calldata positionIdList,
    uint128 positionSize,
    uint64 effectiveLiquidityLimitX32,
    int24 tickLimitLow,
    int24 tickLimitHigh
) external {
    _mintOptions(
        positionIdList,
        positionSize,
        effectiveLiquidityLimitX32,
        tickLimitLow,
        tickLimitHigh
    );
}

function _mintOptions(
    TokenId[] calldata positionIdList,
    uint128 positionSize,
    uint64 effectiveLiquidityLimitX32,
    int24 tickLimitLow,
    int24 tickLimitHigh
) internal {
    TokenId tokenId = positionIdList[positionIdList.length - 1];

    // Validate position list and user state
    _validatePositionList(msg.sender, positionIdList, 1);

    if (tokenId.poolId() != SFPM.getPoolId(address(s_univ3pool)))
        revert Errors.InvalidTokenIdParameter(0);

    // Disallow user to mint exact same position
    if (LeftRightUnsigned.unwrap(s_positionBalance[msg.sender][tokenId]) != 0)
        revert Errors.PositionAlreadyMinted();

    // Mint in the SFPM and update state of collateral
    uint128 poolUtilizations = _mintInSFPMAndUpdateCollateral(
        tokenId,
        positionSize,
        tickLimitLow,
        tickLimitHigh
    );

    _addUserOption(tokenId, effectiveLiquidityLimitX32);

    // Update user's position balance
    s_positionBalance[msg.sender][tokenId] = LeftRightUnsigned
        .wrap(0)
        .toLeftSlot(poolUtilizations)
        .toRightSlot(positionSize);

    // Check solvency after minting with a buffer
    uint256 medianData = _validateSolvency(msg.sender, positionIdList, BP_DECREASE_BUFFER);

    if (medianData != 0) s_miniMedian = medianData;

    emit OptionMinted(msg.sender, positionSize, tokenId, poolUtilizations);
}

Issue with Current Validation:

• The function _mintOptions extracts the last tokenId from positionIdList without checking if the list is non-empty.
• If positionIdList is empty, attempting to access positionIdList[positionIdList.length - 1] will result in an out-of-bounds error, potentially causing the contract to revert unexpectedly.

Proof:

Consider the following scenario:

1. A user calls mintOptions with an empty positionIdList.
2. The function _mintOptions attempts to access positionIdList[positionIdList.length - 1].
3. This results in an out-of-bounds access error, causing the contract to revert.

Tools Used

Manual code review.

Recommended Mitigation Steps

Validate positionIdList:

Ensure positionIdList is non-empty and contains valid TokenIds before processing:

function _mintOptions(
    TokenId[] calldata positionIdList,
    uint128 positionSize,
    uint64 effectiveLiquidityLimitX32,
    int24 tickLimitLow,
    int24 tickLimitHigh
) internal {
    require(positionIdList.length > 0, "PositionIdList cannot be empty");

    TokenId tokenId = positionIdList[positionIdList.length - 1];

    // Validate position list and user state
    _validatePositionList(msg.sender, positionIdList, 1);

    if (tokenId.poolId() != SFPM.getPoolId(address(s_univ3pool)))
        revert Errors.InvalidTokenIdParameter(0);

    // Disallow user to mint exact same position
    if (LeftRightUnsigned.unwrap(s_positionBalance[msg.sender][tokenId]) != 0)
        revert Errors.PositionAlreadyMinted();

    // Mint in the SFPM and update state of collateral
    uint128 poolUtilizations = _mintInSFPMAndUpdateCollateral(
        tokenId,
        positionSize,
        tickLimitLow,
        tickLimitHigh
    );

    _addUserOption(tokenId, effectiveLiquidityLimitX32);

    // Update user's position balance
    s_positionBalance[msg.sender][tokenId] = LeftRightUnsigned
        .wrap(0)
        .toLeftSlot(poolUtilizations)
        .toRightSlot(positionSize);

    // Check solvency after minting with a buffer
    uint256 medianData = _validateSolvency(msg.sender, positionIdList, BP_DECREASE_BUFFER);

    if (medianData != 0) s_miniMedian = medianData;

    emit OptionMinted(msg.sender, positionSize, tokenId, poolUtilizations);
}

Assessed type

Invalid Validation

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid