Underflow in _createLegInAMM

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L979

Vulnerability details

Impact

removedLiquidity -= chunkLiquidity; will underflow when there is no removedLiquidity in a particular leg.

This is a result of the fact that, when a token is being burnt, the tokenId is flipped to change the isLong to from 1 to 0, this happens for all the legs.

The problem with here is that not all legs will be 1, some will retain their zero(0 when no liquidity was previously removed). For this reason, removedLiquidity = 0 for such legs, but it was assumed that once the flag is BURN, removeLiquidity will be positive.

Since the calculation happened in an unchecked block, it won't revert but underflow, increasing removedLiquidity to a very large amount. here

return self ^ ((LONG_MASK >> (48 * (4 - optionRatios))) & CLEAR_POOLID_MASK);

When _createPositionInAMM loops through every leg to create a position in AMM, it calls _createLegInAMM which checks the

Proof of Concept

For a tokenId with two legs for instance, the first leg has isLeg = 1, the second leg has isLong = 0, when flipped, 1 changes to 0, in the first leg, but the second leg remains 0, but removedLiquidity -= chunkLiquidity; is still computed assuming that the zero was as a result of the flipToBurnToken code

/// @dev If the isLong flag is 0=short but the position was burnt, then this is closing a long position
/// @dev so the amount of short liquidity should decrease.
if (_isBurn) {
removedLiquidity -= chunkLiquidity;
}

Tools Used

Manual review.

Recommended Mitigation Steps

Add a check to ensure removedLiquidity >= chunkLiquidity

Assessed type

Math

[c4-judge]

Picodes marked the issue as duplicate of #516

[c4-judge]

Picodes marked the issue as partial-50

[c4-judge]

Picodes marked the issue as partial-25

[Picodes]

No valid scenario or impact are described