Minted positions leg can be override leading to unexpected change in position.

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L547-L561

Vulnerability details

Summary

A user can knowingly or unknowingly override a minted position leg index with a different leg containing a different configuration.

Detailed Description

There are checks actually put in place by the protocol to safe guard against this, but it some how still break through by allowing users to knowingly or unknowingly override a minted position leg index with a different leg containing a different configurations such as the options type, assets and so on. This

Example;

• Bob mints a position by calling the mintOptions function after configuring the legs in the positionIdList array by passing in the required values and starting at an index of 0 tokenId leg.

• AFter a while, bob can either knowingly mint another position still in the positionIdList by configuring a tokenId with a leg and specifying an index of 0 instead of 1 which is meant to be the next legIndex.

• This overrides the existing index of 1 and clears out the previous index of 0 leading to a deletion of that particular position.

Impact:

This is a break in core contracts logic as the protocol was meant to be resilient against such actions.

Proof of Code

After running the following test, check the tokenId.countLegs() and it will be 1 as opposed to actual value of 2

Run the following test in the projects test suit using the same setup to validate the bug

    function test_overridingLegIndex(uint256 x, uint104 amount, uint256 widthSeed, int256 strikeSeed, uint128 positionSizeSeed ) public {
        _initWorld(x);

        // Invoke all interactions with the Collateral Tracker from user Bob
        vm.startPrank(Bob);

        // give Bob the max amount of tokens
        _grantTokens(Bob);

        // approve collateral tracker to move tokens on Bob's behalf
        IERC20Partial(token0).approve(address(collateralToken0), amount);
        IERC20Partial(token1).approve(address(collateralToken1), amount);

        // deposit a number of assets determined via fuzzing
        // equal deposits for both collateral token pairs for testing purposes
        collateralToken0.deposit(amount, Bob);
        collateralToken1.deposit(amount, Bob);

        // call will be minted in range
        (width, strike) = PositionUtils.getOTMSW(
            widthSeed,
            strikeSeed,
            uint24(tickSpacing),
            currentTick,
            1
        );

        // open SELL(short) POSITION as Bob - isLong = 0
        tokenId = TokenId.wrap(0).addPoolId(poolId).addLeg(0, 1, 1, 0, 0, 0, strike, width);
        positionIdList.push(tokenId);

        /// calculate position size
        (legLowerTick, legUpperTick) = tokenId.asTicks(0);
        positionSize0 = uint128(bound(positionSizeSeed, 2, 2 ** 104));
        _assumePositionValidity(Bob, tokenId, positionSize0);
        // Mint the First option
        panopticPool.mintOptions(positionIdList, positionSize0, 0, 0, 0);

        // check the total number of legs the user has
        uint legs = tokenId.countLegs();
        console2.log("no of legs", legs);

        /** Mint another option Buy(long) in the same positionIdList
        */
        tokenId = TokenId.wrap(0).addPoolId(poolId).addLeg(0, 1, 1, 1, 0, 0, strike, width);
        positionIdList.push(tokenId);

        /// calculate position size
        (legLowerTick, legUpperTick) = tokenId.asTicks(0);
        positionSize0 = uint128(bound(positionSizeSeed, 2, 2 ** 104));
        _assumePositionValidity(Bob, tokenId, positionSize0);
        // Mint the second option
        panopticPool.mintOptions(positionIdList, positionSize0, 0, 0, 0);

        // Counted legs will still be equal to 1 because the second mintOption override and deleted the first one
        legs = tokenId.countLegs();
        console2.log("no of legs 2", legs);

        // No of long positions 
        uint longs = tokenId.countLongs();
        console2.log("no of longs", longs);

    }

Tool used:

Manual Review

Recommended Mitigation:

The protocol should make provision for more internal checks in the configuration of tokenIds

Assessed type

Error

[c4-judge]

Picodes marked the issue as primary issue

[dyedm1]

All tokenId operations are supposed to be on empty slots -- there is a clearLeg function that allows a tokenId leg to be cleared if needed. The tokenId is just a 256-bit number and does not in itself carry significant information about the protocol's current state. In your example, you are minting two different tokenIds (which are accounted as two entirely separate positions) that each have one leg/long, and then querying one of them (which should indeed have one leg). Don't see an issue here.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid