Submitted by rscodes, also found by gregom, MrValioBg, McToady, and MrPotatoMagic
In the file PhiFactory.sol, the lack of checks in merkleClaim allows malicious users to bypass the hash check to pass in their own parameter of choice. The 3 variables that could possibly be manipulated (referral, artId and imageURI) and their impact will be each discussed below.
As we can see in merkleClaim, address ref_ and uint256 artId_ is decoded from encodeData_ which is simply a bytes data type passed into the function parameter by the user.
Observing the line where the merkle hash is cross-checked:
It becomes clear that only minter_ from encodeData_ is checked, leaving ref_ and artId_ to whatever values passed by the user.
By the way, both ref_ and artId_ are explicitly checked in the signatureClaim function:
Hence, to forge ref_ and artId_, malicious users will have to use merkleClaim instead of signatureClaim.
Impact of forging ref_
Impact of forging artId_
We can see in both signatureClaim Line 330 and merkleClaim Line 355, imageURI is passed into the function parameter rather than extracted from the signature and is not involved in the merkle hash check as well.
Since imageURI contains important information about the art details (such as location/directory). This could have a severe impact. When the malicious user is trying to claim an art that he has permission to, he could pass in an imageURI with the location directory of the picture image file pointing to another art which he does not have permission for.
The line advancedTokenURI[tokenId_][to_] = imageURI_; in PhiNFT1155.sol will also be set to an unapproved value. This will lead to undefined/unsupported behaviours when interacting with the smart contract regarding trying to render an users art URI.
Add this function to test/PhiFactory.t.sol:TestPhiFactory:
Run forge test --match-test test_claimHack -vv:
As you can see from the Code and the output, Alice managed to set the referral address to her second account (Alice_2), and received the referral reward of 0.00005 ether; which will be a significant sum if Alice was minting high quantity of art, since rewards are calculated by quantity_ * referralReward
Alice also managed to manipulate image_uri, encoding IMAGE_URL2 instead of IMAGE_URL into the function parameter. Both IMAGE_URL2 and IMAGE_URL are different variables which are already declared in the original foundry test file.
In function merkleClaim:
In function signatureClaim:
Foundry, VSCode
ZaK3939 (Phi) confirmed and commented:
0xDjango (judge) decreased severity to Medium and commented:
