Premia owed Skipped for Long Positions with No Current Liquidity in pool

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L936
https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L1200

Vulnerability details

Vulnerability Details:

In the protocol, uniswap fees and premia for a position's legs are calculated within the _collectAndWritePositionData function, which is invoked by _createLegInAMM. This latter function handles both minting and burning of token positions. The protocol performs a check for existing liquidity before collecting accumulated fees, if there isn’t any it skips over the function.

function _createLegInAMM(
        ...
    ) internal returns (int256 _moved, int256 _itmAmounts, int256 _totalCollected) {
	...

        // if there was liquidity at that tick before the transaction, collect any accumulated fees
        if (currentLiquidity.rightSlot() > 0) {
            _totalCollected =
                _collectAndWritePositionData(_liquidityChunk, _univ3pool, currentLiquidity, positionKey, _moved, isLong);
        }

        // position has been touched, update s_accountFeesBase with the latest values from the pool.positions
        s_accountFeesBase[positionKey] = _getFeesBase(_univ3pool, updatedLiquidity, _liquidityChunk);
    }

However, an issue exists in the case of creating a long position (isLong == 1), where the entire current liquidity is used up, leading to a currentLiquidity.rightSlot() being zero:

function _createLegInAMM(
        ...
    ) internal returns (int256 _moved, int256 _itmAmounts, int256 _totalCollected) {
	...
        unchecked {
            uint128 startingLiquidity = currentLiquidity.rightSlot();
            uint128 removedLiquidity = currentLiquidity.leftSlot();
            uint128 chunkLiquidity = _liquidityChunk.liquidity();

		...
                if (startingLiquidity < chunkLiquidity) {
                    revert Errors.NotEnoughLiquidity();
                } else {
                    // we want to move less than what already sits in uniswap, no problem:
                    updatedLiquidity = startingLiquidity - chunkLiquidity;
                    // @audit-note we can create a long using up all liq in the pool: currentLiquidity.rightSlot() = 0
                }
		    ...
            }

In this scenario, the next check for existing liquidity in _createLegInAMM would skip the _collectAndWritePositionData function. This is fine for the uniswap fees as no additional fees would have been accumulated with zero liquidity in the pool, however since _collectAndWritePositionData deals with the premia owed as well, the open long’s owed premia will not be accounted for in that time slot.

Impact:

• Severity: High. The protocol fails to account for premia owed by users in this specific scenario, leading to potential imbalances in the protocol's balances and a loss of fees.
• Likelihood: Low. The issue only works under particular conditions (long positions with no liquidity in the pool), reducing the frequency of occurrence.

Tools Used:

Manual analysis

Recommendation:

The protocol should revise its logic to ensure that premia owed by users are consistently accounted for, regardless of the liquidity status in a position.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #211

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)