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.
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 ofpositionIdList
. So the main logic involves iterating through tokenIds and performing XOR operations, then comparing the result withs_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 againstMAX_POSITIONS
which is32
instead of255
: https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/PanopticPool.sol#L1371-L1384Impact
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