Snowman Merkle Airdrop

[High] Merkle Proof Binding to Balance is Dangerous in SnowmanAirdrop.sol::claimSnowman()

Description

The Merkle leaf in the SnowmanAirdrop.sol::claimSnowman() function is computed using the dynamic on-chain balance of i_snow tokens for the receiver. Specifically, the amount variable, derived from i_snow.balanceOf(receiver), is directly used in the Merkle leaf generation:
javascript uint256 amount = i_snow.balanceOf(receiver); bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(receiver, amount))));
This means the Merkle proof is being verified against a mutable, real-time balance rather than the static amount from the snapshot used to build the Merkle tree off-chain.

Risk

This design introduces critical issues and makes the Merkle proof mechanism unreliable and vulnerable:
1. Denial of Claims for Legitimate Users: If a user's i_snow balance changes (e.g., they transfer tokens out, or receive more tokens) after the Merkle tree snapshot was taken, the amount derived from balanceOf at the time of claiming will no longer match the amount used to generate their original leaf. This mismatch will cause the MerkleProof.verify function to fail, preventing legitimate users from claiming their tokens.
2. Potential for Malicious Exploitation (in conjunction with H-6): While the current checks might limit direct double-spending based on proof alone, combined with the unsafe receiver as signer (H-6), an attacker could potentially find an eligible (receiver, amount) pair from the Merkle tree and attempt to exploit timing or balance changes to create favorable conditions for their claims or to block others.

Proof of Concept

The explicit use of uint256 amount = i_snow.balanceOf(receiver); immediately before computing bytes32 leaf = keccak256(abi.encode(receiver, amount)); highlights the dependency on mutable on-chain state for Merkle proof verification.

Recommended Mitigation

The Merkle proof must be computed from immutable, off-chain data that was snapshotted at a specific time.

1. Require amount as an Explicit Argument: The amount that was included in the off-chain snapshot (and used to generate the Merkle proof) should be passed as an explicit argument to the claimSnowman function. This amount should also be included in the EIP-712 signed message (refer to H-6).

2. Merkle Leaf from Immutable Data: The Merkle leaf computation within the contract must use this explicitly passed expectedAmount argument, which represents the immutable snapshot data. The i_snow.balanceOf(receiver) should not be used for Merkle leaf generation. If a minimum current balance is still a project requirement, it should be a separate require check after the Merkle proof validation (e.g., require(i_snow.balanceOf(receiver) >= expectedAmount, "SA__InsufficientCurrentBalance");), but understand this can still lead to legitimate users being unable to claim if their balance fluctuates.

   // Assume `expectedAmount` is now passed as an argument to the function

   function claimSnowman(address receiver, uint256 expectedAmount, bytes32[] calldata merkleProof, uint8 v, bytes32 r, bytes32 s, uint256 nonce)

   external

   nonReentrant

   {

   // ... (previous checks for `!s_hasClaimedSnowman[receiver]` and `!s_usedNonces[nonce]`)

   ​

   if (receiver == address(0)) {

   revert SA__ZeroAddress();

   }

   ​

   // Removed the problematic `uint256 amount = i_snow.balanceOf(receiver);` line.

   // The `expectedAmount` from the function arguments is used.

   ​

   // Merkle leaf MUST use the `expectedAmount` that was part of the original snapshot data.

   bytes32 leaf = keccak256(abi.encode(receiver, expectedAmount)); // Corrected: Using expectedAmount

   ​

   if (!MerkleProof.verify(merkleProof, i_merkleRoot, leaf)) {

   revert SA__InvalidProof();

   }

   ​

   // Signature validation now also uses expectedAmount and binding to msg.sender/nonce

   if (!_isValidSignature(receiver, getMessageHash(receiver, expectedAmount, nonce, msg.sender), v, r, s)) {

   revert SA__InvalidSignature();

   }

   ​

   // Optional: If you still need a current balance check (careful, can fail legitimate claims)

   // require(i_snow.balanceOf(receiver) >= expectedAmount, "SA__InsufficientCurrentBalanceForClaim");

   ​

   i_snow.safeTransferFrom(receiver, address(this), expectedAmount);

   s_usedNonces[nonce] = true;

   s_hasClaimedSnowman[receiver] = true;

   ​

   emit SnowmanClaimedSuccessfully(receiver, expectedAmount);

   ​

   i_snowman.mintSnowman(receiver, expectedAmount);

   }