_validatePositionList() positionIdList can forgery #463
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/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1367
Vulnerability details
Vulnerability details
_validatePositionList(positionIdList)
is called from multiple places and is primarily used to validate the legality ofpositionIdList
.The main logic involves iterating through
tokenId
s and performing XOR operations, then comparing the result withs_positionsHash[account]
.The main issue in the code is with
(((existingHash >> 248) + 1) << 248)
. The firstuint8
represents the number oftokenId
s, and overflow is ignored.Additionally,when
XOR
calculation, the known formula isX^Y^Y=X
.This means that calculating the hash for
positionIdList[1,2,3]
andpositionIdList[1,2,3,4,4,4,4......]
(2564
) will result in the same hash.Proof of Concept
The following code demonstrates that inserting any
256
tokenId
s will yield the same hash.Impact
If a
seller
has atokenId
premium
>required collateral
, they can maliciously increasepositionIdList
by adding any256*N
number oftokenId
s, passing the validation in_validateSolvency()
, and thus illegally opening a position.Recommended Mitigation
function updatePositionsHash( uint256 existingHash, TokenId tokenId, bool addFlag ) internal pure returns (uint256) { // add the XOR`ed hash of the single option position `tokenId` to the `existingHash` // @dev 0 ^ x = x unchecked { // update hash by taking the XOR of the new tokenId uint248 updatedHash = uint248(existingHash) ^ (uint248(uint256(keccak256(abi.encode(tokenId))))); // increment the top 8 bit if addflag=true, decrement otherwise + require((existingHash >> 248)<255,"invald"); return addFlag ? uint256(updatedHash) + (((existingHash >> 248) + 1) << 248) : uint256(updatedHash) + (((existingHash >> 248) - 1) << 248); } }
Assessed type
Context
The text was updated successfully, but these errors were encountered: