VerificationType is not enforced for claiming NFT directly with signatureClaim()

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/PhiFactory.sol#L327-L346

Vulnerability details

Impact

When an artist creates art, they can set the verificationType. If the verificationType is set to "Signature", claims using merkle trees directly via merkleClaim() or claim() cannot be executed, as no merkleRoot is expected to be added when creating art.

However, the flow is missing a check in signatureClaim(), so it's possible to use both MERKLE and SIGNATURE for proof when verificationType is set to MERKLE.

Proof of Concept

Check here how validationType is set upon art creation.

We can see how its enforced, when using claim():

    function claim(bytes calldata encodeData_) external payable {
        bytes memory encodedata_ = LibZip.cdDecompress(encodeData_);
        (uint256 artId) = abi.decode(encodedata_, (uint256));
        PhiArt storage art = arts[artId];
        uint256 tokenId = IPhiNFT1155Ownable(art.artAddress).getTokenIdFromFactoryArtId(artId);
        
@>>   //@audit enforcing type based on the verificationType saved in the art
@>>    if (art.verificationType.eq("MERKLE")) {
         .....
@>>     } else if (art.verificationType.eq("SIGNATURE")) {
         .....
        } else {
            revert InvalidVerificationType();
        }
    }

Its also enforced when we use merkleClaim :

    function merkleClaim(....)  {
        (address minter_, address ref_, uint256 artId_) = abi.decode(encodeData_, (address, address, uint256));
        PhiArt storage art = arts[artId_];

        bytes32 credMerkleRootHash = credMerkleRoot[art.credChainId][art.credId];
@>>     if (minter_ == address(0) || !art.verificationType.eq("MERKLE") || credMerkleRootHash == bytes32(0)) {
            revert InvalidMerkleProof();
        }
       ......

However, such verification does not exist in signatureClaim() allowing us to claim with both signatures and merkle when calling signatureClaim() instead of claim(), even though the verificationType is explicitly set to "MERKLE":

Verify with code-snippet here

Tools Used

Manual Review

Recommended Mitigation Steps

Enforce verificationType with a explicit check:

    function signatureClaim(
        bytes calldata signature_,
        bytes calldata encodeData_,
        MintArgs calldata mintArgs_
    )
        external
        payable
        whenNotPaused
    {
        (uint256 expiresIn_, address minter_, address ref_, address verifier_, uint256 artId_,, bytes32 data_) =
            abi.decode(encodeData_, (uint256, address, address, address, uint256, uint256, bytes32));
+       PhiArt storage art = arts[artId];
+       if (!art.verificationType.eq("SIGNATURE")) revert;
        
        if (expiresIn_ <= block.timestamp) revert SignatureExpired();
        if (_recoverSigner(keccak256(encodeData_), signature_) != phiSignerAddress) revert AddressNotSigned();

        _validateAndUpdateClaimState(artId_, minter_, mintArgs_.quantity);
        _processClaim(artId_, minter_, ref_, verifier_, mintArgs_.quantity, data_, mintArgs_.imageURI, msg.value);

        emit ArtClaimedData(artId_, "SIGNATURE", minter_, ref_, verifier_, arts[artId_].artAddress, mintArgs_.quantity);
    }

Assessed type

Context