Panoptic - lanrebayode77's results

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

L1 Incorrect/misleading comment on TokenId example

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/types/TokenId.sol#L52

///  - the underlying strike price of the 2nd leg (leg index = 1) in this option position starts at bit index  (64 + 12 + 48 * (leg index=1))=123

By specification the strike price of the second leg will commence at bit index 124

--- ///  - the underlying strike price of the 2nd leg (leg index = 1) in this option position starts at bit index  (64 + 12 + 48 * (leg index=1))=123
+++ ///  - the underlying strike price of the 2nd leg (leg index = 1) in this option position starts at bit index  (64 + 12 + 48 * (leg index=1))=124 

#L2 Redundant casting https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L1381 The variable came in form of an address, so there is no need to cast it in an address type

        bytes32 positionKey = keccak256(
          ---  abi.encodePacked(address(univ3pool), owner, tokenType, tickLower, tickUpper)
          +++  abi.encodePacked(univ3pool, owner, tokenType, tickLower, tickUpper)
        );