Hash collision in the PanopticMath.updatePositionsHash
function could prompt wrong positions in the positionIdList
to be validated
#496
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
downgraded by judge
Judge downgraded the risk level of this issue
duplicate-498
🤖_178_group
AI based duplicate group recommendation
satisfactory
satisfies C4 submission criteria; eligible for awards
Lines of code
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/libraries/PanopticMath.sol#L100-L109
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1382-L1394
Vulnerability details
Impact
The
PanopticPool._validatePositionList
is a critical function in thePanoptic protocol
. This function is used to ensure that the positions in the incoming user's list match the existing active option positions.The
_validatePositionList
function is used by multiple critical functions in the protocol such as_mintOptions
,_validateSolvency
,liquidate
,forceExercise
andsettleLongPremium
. These functions ensure the solvency of the protocol and hence can be considered as highly critical functions for the health and sustainability of the protocol.The
PanopticPool._validatePositionList
function calls thePanopticMath.updatePositionsHash
in its execution flow. ThePanopticMath.updatePositionsHash
function is used to update an existing account's "positions hash" with a new single positiontokenId
. This updated position hash is calculated as shown below:But there is an issue with the above calculation since the
hashed
value of theencoded tokenId
, by thekeccak256
is truncated to a uint248 (from uint256) before performingXOR
with theexistingHash
. This truncation is performed to keep themost significant 8 bytes
to store thecount of the positions
.Due to the above truncation a hash collision could occur where two different
tokenIds
with two different option positions could result in the same truncateduint248
hash value. As a result a malicious user can provide amanipulated tokenId inplace of a valid tokenId
in thepositionIdList
, as an input to thePanopticPool._validatePositionList
function such that when truncated (hash value) touint248
, the resultingupdatedHash
would equal to theexisting active option positions
stored in thes_positionsHash[account]
mapping (stored in the storage).As it was explained earlier since the
_validatePositionList
is used by critical functions such as_mintOptions
,_validateSolvency
,liquidate
,forceExercise
andsettleLongPremium
, the above vulnerability could create many undesired behaviors in the protocol and could break the protocol.For example during the
liquidate
function call a malicious liquidator can use the above vulnerability to provide maliciouspositionIdListLiquidator
andpositionIdList
as inputTokenId[]
arrays to thePanopticPool.liquidate
function. If the malcious tokenId or tokenIds result in the same truncated hash as of the valid tokenIds in the storage option position, the transaction will proceed without revert.As a result the malicious liquidator can
liquidate
a position with different position parameters for a givenliquidatee
which is an undesirable state to the protocol. This could lead to more liquidity being removed or added to the UniswapV3 pool than the valid position truly hold. This could further lead to broken state in the fee calculation as well. Further to the above amalicious liquidator
can bypass the_checkSolvencyAtTick
check in theliquidate
function by providingmalicoius tokenIds
which would give himenough balance
to stay solvent. This happens because the preceeding_validatePositionList
check will not be able to identify themalicious tokenId
in thepositionIdListLiquidator
array since the truncated hash (to uint248) matches the valid tokenId hash, stored in the storage (s_positionsHash[account]
) for the given tokenId position.The above vulnerability could create such undesired behavior and broken state in the other critical functions I explained above as well. This could lead the panoptic protocol to be insolvent as well.
Proof of Concept
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/libraries/PanopticMath.sol#L100-L109
https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/PanopticPool.sol#L1382-L1394
Tools Used
Manual Review and VSCode
Recommended Mitigation Steps
Hence it is recommended to use the complete hash value of the
tokenId
(uint256) whenupdatedHash
is calculated in thePanopticMath.updatePositionsHash
function. This will ensure no hash collision will occur for two differenttokenIds
. To save thecount of the positions
a separate state variable can be used in thePanopticPool
contract.Assessed type
Other
The text was updated successfully, but these errors were encountered: