Eligible users can claim their airdrop amounts over and over again, draining the contract

Severity

High Risk

Description

A user eligible for the airdrop can verify themselves as being part of the merkle tree and claim their airdrop amount. However, there is no mechanism enabled to track the users who have already claimed their airdrop, and the merkle tree is still composed of the same user. This allows users to drain the MerkleAirdrop contract by calling the MerkleAirdrop::claim() function over and over again.

Impact

Severity: High
Likelihood: High

A malicious user can call the MerkleAirdrop::claim() function over and over again until the contract is drained of all its funds. This also means that other users won't be able to claim their airdrop amounts.

Proof of Code

Add the following test to ./test/MerkleAirdrop.t.sol,

    function testClaimAirdropOverAndOverAgain() public {
        vm.deal(collectorOne, airdrop.getFee() * 4);

        for (uint8 i = 0; i < 4; i++) {
            vm.prank(collectorOne);
            airdrop.claim{ value: airdrop.getFee() }(collectorOne, amountToCollect, proof);
        }

        assertEq(token.balanceOf(collectorOne), 100e6);
    }

The test passes, and the malicious user has drained the contract of all its funds.

Tools Used

Manual review and Foundry.

Recommended Mitigation

Use a mapping to store the addresses that have claimed their airdrop amounts. Check and update this mapping each time a user tries to claim their airdrop amount.

contract MerkleAirdrop is Ownable {
    using SafeERC20 for IERC20;

    error MerkleAirdrop__InvalidFeeAmount();
    error MerkleAirdrop__InvalidProof();
    error MerkleAirdrop__TransferFailed();
+   error MerkleAirdrop__AlreadyClaimed();

    uint256 private constant FEE = 1e9;
    IERC20 private immutable i_airdropToken;
    bytes32 private immutable i_merkleRoot;
+   mapping(address user => bool claimed) private s_hasClaimed;

    ...

    function claim(address account, uint256 amount, bytes32[] calldata merkleProof) external payable {
+       if (s_hasClaimed[account]) revert MerkleAirdrop__AlreadyClaimed();
        if (msg.value != FEE) {
            revert MerkleAirdrop__InvalidFeeAmount();
        }
        bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(account, amount))));
        if (!MerkleProof.verify(merkleProof, i_merkleRoot, leaf)) {
            revert MerkleAirdrop__InvalidProof();
        }
+       s_hasClaimed[account] = true;
        emit Claimed(account, amount);
        i_airdropToken.safeTransfer(account, amount);
    }

Now, let's unit test the changes,

    function testCannotClaimAirdropMoreThanOnceAnymore() public {
        vm.deal(collectorOne, airdrop.getFee() * 2);

        vm.prank(collectorOne);
        airdrop.claim{ value: airdrop.getFee() }(collectorOne, amountToCollect, proof);

        vm.prank(collectorOne);
        airdrop.claim{ value: airdrop.getFee() }(collectorOne, amountToCollect, proof);
    }

The test correctly fails, with the following logs,

Failing tests:
Encountered 1 failing test in test/MerkleAirdropTest.t.sol:MerkleAirdropTest
[FAIL. Reason: MerkleAirdrop__AlreadyClaimed()] testCannotClaimAirdropMoreThanOnceAnymore() (gas: 96751)