PanopticPool._validatePositionList can be tricked by overflowing position counter

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/libraries/PanopticMath.sol#L100-L109
https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1367

Vulnerability details

Impact

A malicious actor can forge the currently held positions list and trick the system into believing that, for example, their overall position is solvent.

Proof of Concept

The user's currently held positions list is not stored on-chain, instead, it is provided with each mintOptions/burnOptions call, hashed, and then compared to the stored hash (fingerprint) in _validatePositionList.

function _validatePositionList(
    address account,
    TokenId[] calldata positionIdList,
    uint256 offset
) internal view {
    uint256 pLength;
    uint256 currentHash = s_positionsHash[account];
    unchecked {
        pLength = positionIdList.length - offset;
    }
    uint256 fingerprintIncomingList;
    for (uint256 i = 0; i < pLength; ) {
        fingerprintIncomingList = PanopticMath.updatePositionsHash(
            fingerprintIncomingList,
            positionIdList[i],
            ADD
        );
        unchecked {
            ++i;
        }
    }
    if (fingerprintIncomingList != currentHash) revert Errors.InputListFail();
}

updatePositionsHash XORes keccak of each token in right 248 bits and stores positions count in left 8 bit.

function updatePositionsHash(
    uint256 existingHash,
    TokenId tokenId,
    bool addFlag
) internal pure returns (uint256) {
    unchecked {
        uint248 updatedHash = uint248(existingHash) ^ (uint248(uint256(keccak256(abi.encode(tokenId)))));
        return
            addFlag
                ? uint256(updatedHash) + (((existingHash >> 248) + 1) << 248)
                : uint256(updatedHash) + (((existingHash >> 248) - 1) << 248);
    }
}

This validation can be bypassed when addFlag=true by overflowing the counter and hiding tokens by doubling (x ^ x = 0, x ^ 0 = x). For example, assuming counter has only 2 bits:
[x, x, x, x, a] => x ^ x ^ x ^ x ^ a | 1 + 1 + 1 + 1 + 1 => a | 1.

For coded PoC make the following changes in, for example, test_Success_mintOptions_OTMShortCall:

- TokenId[] memory posIdList = new TokenId[](1);
- posIdList[0] = tokenId;
+ TokenId[] memory posIdList = new TokenId[](257);
+ for (uint i; i < 257; i++) posIdList[i] = tokenId;

Validating position list will pass meaning we successfully "hide" 256 tokens from the validator, but they still counted in _validateSolvency and other functions.

Tools Used

Manual review

Recommended Mitigation Steps

Enforce MAX_POSITIONS limit in _validatePositionList by adding require: require(positionIdList.length <= MAX_POSITIONS)

Assessed type

Invalid Validation

[c4-judge]

Picodes marked the issue as duplicate of #498

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 2 (Med Risk)