howlbot-integration[bot] commented 5 months ago

Lines of code

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

Vulnerability details

Proof of Concept

First see the previous issue.

Take a look at https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/PanopticPool.sol#L1334-L1361

TLDR of the issue is that the _validatePositionList() function is used multiple places and is primarily used to validate the legality of positionIdList. So the main logic involves iterating through tokenIds and performing XOR operations, then comparing the result with s_positionsHash[account], however previously the overflow was ignored, the suggested fix was tol include axcheck that the returned value is less than 255, but the fix implemented as seen below have the check against MAX_POSITIONS which is 32 instead of 255: https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/PanopticPool.sol#L1371-L1384

    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;
    }

Impact

M-02 has not been correctly mitigated, leaving it vulnerable to the same attacks as hinted in the previous issue and it's duplicates

Recommended Mitigation Steps

Apply the suggested fix from the report, which is †hat the check should be against 255 instead.

Assessed type

Context

Picodes commented 5 months ago

The check is now done with https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/libraries/PanopticMath.sol#L143

c4-judge commented 5 months ago

Picodes marked the issue as unsatisfactory: Invalid

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