search

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

1 stars 0 forks source link

Incorrect Validation in _updatePositionsHash Function Allows Exceeding Maximum Positions Limit by One #8

Closed howlbot-integration[bot] closed 1 month ago

howlbot-integration[bot] commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/main/contracts/PanopticPool.sol#L1371

Vulnerability details

Impact

The current implementation of the _updatePositionsHash function in the PanopticPool contract has an incorrect validation check for the maximum number of positions allowed per user. This oversight allows users to exceed the intended maximum positions limit by one. This could lead to unintended behavior and potential system vulnerabilities due to more positions being open than allowed.

Proof of Concept

Lines of Code

function _updatePositionsHash(address account, TokenId tokenId, bool addFlag) internal {
    uint256 newHash = PanopticMath.updatePositionsHash(
        s_positionsHash[account],
        tokenId,
        addFlag
    );
    if ((newHash >> 248) > MAX_POSITIONS) revert Errors.TooManyPositionsOpen();
    s_positionsHash[account] = newHash;
}

In this code, the check (newHash >> 248) > MAX_POSITIONS is intended to ensure that the number of positions does not exceed MAX_POSITIONS. However, this condition will only trigger if the number of positions exceeds MAX_POSITIONS by more than one.

Issue with Current Condition:

Proof:

Consider the following scenario with MAX_POSITIONS set to 32:

  1. A user has 32 positions.
  2. The newHash value after adding another position results in newHash >> 248 equal to 33.
  3. The condition (newHash >> 248) > 32 will not revert, allowing the user to add the 33rd position.

Tools Used

Manual

Recommended Mitigation Steps

To strictly enforce the maximum number of positions, the condition should be changed to >=:

function _updatePositionsHash(address account, TokenId tokenId, bool addFlag) internal {
    uint256 newHash = PanopticMath.updatePositionsHash(
        s_positionsHash[account],
        tokenId,
        addFlag
    );
    if ((newHash >> 248) >= MAX_POSITIONS) revert Errors.TooManyPositionsOpen();
    s_positionsHash[account] = newHash;
}

Assessed type

Invalid Validation

Picodes commented 1 month ago

32 is meant to be a valid input

c4-judge commented 1 month ago

Picodes marked the issue as unsatisfactory: Invalid