Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1405-L1417 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L547-L561 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L614-L667 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1367-L1395 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L739-L782 https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L859-L878

Vulnerability details

Impact

The function _updatePositionsHash does not validate account address, tokenId and the boolean flag indicating whether to add or remove a position. For account address and the boolean flag this is mitigated through the functions that call _updatePositionsHash. But although there are some checks for TokenId, a crucial check is missing, namely the one that validates the correctness of the data encoded within the TokenId.

function _updatePositionsHash(address account, TokenId tokenId, bool addFlag) internal { // Get the current position hash value (fingerprint of all pre-existing positions created by '_account') // Add the current tokenId to the positionsHash as XOR'd // since 0 ^ x = x, no problem on first mint // Store values back into the user option details with the updated hash (leaves the other parameters unchanged) uint256 newHash = PanopticMath.updatePositionsHash( s_positionsHash[account], tokenId, addFlag ); if ((newHash >> 248) > MAX_POSITIONS) revert Errors.TooManyPositionsOpen(); s_positionsHash[account] = newHash; }

In the minting process: if TokenId is not correctly validated, there could be misrepresentation in the encoding of option specifics like strike prices and other parameters. Since the _updatePositionsHash function relies on the TokenId to update the hash representing all active positions for an account, any incorrect TokenId can corrupt this hash. This can make the tracking of which positions are actually open or closed inaccurate.

In the burning process: incorrect TokenId input during the burning process could lead to the wrongful closure of positions. If the burning process does not accurately reflect the current state of an option due to incorrect TokenId usage, users could incur unexpected financial losses. This also opens up a possible Attack Vector: An attacker could pass incorrect tokenId values during the minting or burning processes. Since the function does not validate the tokenId, this would lead to the hash being incorrectly updated. In this case, if the position hash does not accurately reflect the open and closed positions, this can lead to the system believing that certain positions are closed when they are not or the opposite. These misrepresented positions would then lead to wrong collateral calculations, allowing users to withdraw more funds than they are entitled to.

Proof of Concept

Since _updatePositionsHash is internal, it can only be called using two call paths:

first one being: mintOptions (also no input validation) - _mintOptions (_validatePositionList) - _addUserOption - _updatePositionsHash

second one is: burnOptions - _burnOptions - _updatePositionDataBurn - _updatePositionsHash

In the first call path, the user calls the external function mintOptions which then calls _mintOptions , which calls _addUserOption which lastly calls_updatePositionsHash.

msg.sender (account address) is automatically provided and represents the address from which the current function call is made, so the account address can not be manipulated even though the account address is not validated in _updatePositionsHash.

The boolean flag also cannot be manipulated directly from user input calling mintOptions since this flag appears to be hard-coded as true in the _addUserOption function:

_updatePositionsHash(msg.sender, tokenId, ADD);

It follows that normal users cannot change the addFlag from external calls since it's set within the internal logic of the contract and not exposed as a parameter of the external mintOptions function or its internal calls.

But what about the validation of tokenID?

Ideally, validation should occur in _mintOptions, before it's used or passed down to _addUserOption since the functions before don't validate tokenID. The _mint function does perform some checks: it verifies that the tokenId belongs to the correct Panoptic pool. This is done here:

if (tokenId.poolId() != SFPM.getPoolId(address(s_univ3pool))) revert Errors.InvalidTokenIdParameter(0); The function also checks if the tokenId is already minted and active in the user's balance:

if (LeftRightUnsigned.unwrap(s_positionBalance[msg.sender][tokenId]) != 0) revert Errors.PositionAlreadyMinted();

This prevents a user from minting the same position multiple times without first burning or closing the existing position and ensures that each token ID represents a unique position at any given time. But: The _mint function doesn't perform any checks on the correctness of the data encoded within the TokenId. The _mint function then calls the _validatePositionList, which also neglects to perform a check on the correctness of the data encoded in TokenID.

How are TokenIds added and removed anyway? In Panoptic Pool, a new TokenId is added through the functions Minting Options (mintOptions and _mintOptions functions), our first call path: mintOptions (also no input validation) - _mintOptions (_validatePositionList) - _addUserOption - _updatePositionsHash

When a new option position is minted, a TokenId is generated and managed within the contract. This token ID represents specific characteristics and conditions of the option position.

It is removed through the (burnOptions functions), our second call path.: burnOptions - _burnOptions - _updatePositionDataBurn - _updatePositionsHash

These functions allow for the removal (burning) of options identified by their TokenId. During the burn process, the associated TokenId data is updated or removed accordingly. The burnOptions function starts the process of burning or settling options by calling _burnOptions, which adjusts tick limits for current market conditions, retrieves the position size, and handles the burning and calculation of outcomes such as owed premiums. This function then calls _updatePositionDataBurn to clear option data and reset balances. It ensures that closing the option doesn't exceed allowed market spreads and finally updates the hash of positions associated with the account by calling _updatePositionsHash. This last function modifies the internal representation of the user’s positions without validating the tokenId leading to potential risks of data corruption or manipulation.

Using an incorrect tokenId could lead to inaccurate representation of open or closed positions in the user's hash. The consequences would be: Positions might appear closed in the system when they are actually still open, or the other way around. If the burning process does not accurately reflect the current state of an option due to incorrect TokenId usage,a user might think they have closed a position and mitigated losses,while this is not true.

Tools Used

Manual inspection.

Recommended Mitigation Steps

Implement better validation checks for TokenId at every interaction point within the contract. This includes not only the existence checks but also verifying the correctness of the data encoded within the TokenId, especially in _updatePositionsHash, but not only. TokenId validation should be integrated into both the minting and burning processes. In the minting process, validate TokenId before it is added to the position list and used in any hash updates. In the burning process, make sure that the TokenId reflects the actual state of an option before it is removed, preventing incorrect closure or modification of positions. Update the _validatePositionList function to not only check for the matching of hashes but to also validate the integrity and appropriateness of each TokenId included in the position list.

Assessed type

Invalid Validation

code-423n4 / 2024-04-panoptic-findings

Insufficient input validation of TokenId in _updatePositionsHash #326