ctf_sec - Fund in airdrop distribution contract can be stolen because the claim type is not in the merkle tree schema

[sherlock-admin4]

ctf_sec

High

Fund in airdrop distribution contract can be stolen because the claim type is not in the merkle tree schema

Summary

the merkle claim schema is:

   struct MerkleClaim {
        address recipient;
        address target;
        uint tokenId;
        uint amount;
    }

and the logic for claim airdrop is

function claimAirdrop(bytes32 _merkle, Enums.ClaimType _claimType, MerkleClaim calldata _node, bytes32[] calldata _merkleProof) public {

        // @audit _claimType not in the signature schema 
        // Ensure the merkle root exists
        if (!merkleClaims[_merkle][_claimType]) revert MerkleRootNotValid();

        // Hash our node
        bytes32 nodeHash = keccak256(abi.encode(_node));

        // Ensure that the user has not already claimed the airdrop
        if (isClaimed[_merkle][nodeHash]) revert AirdropAlreadyClaimed();

        // Encode our node based on the MerkleClaim and check that the node is
        // valid for the claim.
        if (!MerkleProofLib.verifyCalldata(_merkleProof, _merkle, nodeHash))
            revert InvalidClaimNode();

        // Mark our merkle as claimed against by the recipient
        isClaimed[_merkle][nodeHash] = true;

        // Check the claim type we are dealing with and distribute accordingly
        if (_claimType == Enums.ClaimType.ERC20) {
            if (!IERC20(_node.target).transfer(_node.recipient, _node.amount)) revert TransferFailed();
        } else if (_claimType == Enums.ClaimType.ERC721) {
            IERC721(_node.target).transferFrom(address(this), _node.recipient, _node.tokenId);
        } else if (_claimType == Enums.ClaimType.ERC1155) {
            IERC1155(_node.target).safeTransferFrom(address(this), _node.recipient, _node.tokenId, _node.amount, '');
        } else if (_claimType == Enums.ClaimType.NATIVE) {
            (bool sent,) = payable(_node.recipient).call{value: _node.amount}('');
            if (!sent) revert TransferFailed();
        }

        emit AirdropClaimed(_merkle, _claimType, _node);
    }

the root cause is that the claim type is not in the node schema.

        bytes32 nodeHash = keccak256(abi.encode(_node));

consider the case:

the airdrop contract hold 100 ETH and 10000 USDC.

the user is entitled to 100 USDC airdrop, the user get the merkle proof and the MerkleClaim

both

consider the case Enums.ClaimType.NATIVE and Enums.ClaimType.ERC20 is enabled.

then user while user should input Enums.ClaimType.ERC20 and get the 100 USDC airdrop,

user can use the same claim node to input Enums.ClaimType.NATIVE and steal all 100 ETH.

else if (_claimType == Enums.ClaimType.NATIVE) {
            (bool sent,) = payable(_node.recipient).call{value: _node.amount}('');
            if (!sent) revert TransferFailed();
        }

Vulnerability Detail

Impact

lose of fund

Code Snippet

https://github.com/sherlock-audit/2024-08-flayer/blob/0ec252cf9ef0f3470191dcf8318f6835f5ef688c/flayer/src/contracts/utils/AirdropRecipient.sol#L121

Tool used

Manual Review

Recommendation

Change the code to include the claim type in the merkle tree node schema.

        // Hash our node
        bytes32 nodeHash = keccak256(abi.encode(_node, claimType));

[ZeroTrust01]

https://github.com/sherlock-audit/2024-08-flayer/blob/0ec252cf9ef0f3470191dcf8318f6835f5ef688c/flayer/src/contracts/utils/AirdropRecipient.sol#L102 Because this is an admin-operated function, it cannot be assumed that the admin mistakenly set the same _merkle to true for different _claimType. The _merkle and _claimType are one-to-one correspondences, so it cannot be assumed that a single _merkle contains two or more _claimType for airdrops. Therefore, if (!merkleClaims[_merkle][_claimType]) revert MerkleRootNotValid(); will revert.

The claim type is better defined in the merkle tree schema. This issue is Low/Info.

[MrValioBg]

Actually @ZeroTrust01 I believe the statement "it cannot be assumed that the admin mistakenly set the same _merkle to true for different _claimType." could be wrong, because as described here https://github.com/sherlock-audit/2024-08-flayer-judging/issues/332 We have a test that shows that its 100% intended to have multiple ClaimTypes for the same merkle root, example - test_CannotDistributeExistingMerkle()

Edit: regarding the rules tests are indeed not considered source of truth and I agree this issue should be invalid