Snowman Merkle Airdrop

First Flight #42
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Impact: high
Likelihood: high
Invalid

Incorrect `SnowmanAirdrop.sol::claimSnowman()` function `safeTransferFrom` Parameters and Misunderstanding of Claim Flow

[H-8] Incorrect SnowmanAirdrop.sol::claimSnowman() function safeTransferFrom Parameters and Misunderstanding of Claim Flow


Description

The line i_snow.safeTransferFrom(receiver, address(this), amount) in SnowmanAirdrop.sol::claimSnowman() attempts to transfer amount of i_snow tokens from the receiver to the SnowmanAirdrop contract itself. This contradicts the common user expectation of a "claim" function, where a user typically receives tokens without having to send their existing tokens.

* **Problem 1: `receiver` as `from`:** For `safeTransferFrom` to execute successfully, the `receiver` address must have previously granted an ERC20 `approve()` allowance to the `SnowmanAirdrop` contract to spend their `i_snow` tokens. Without this explicit prior approval, the `safeTransferFrom` call will revert, making the core functionality unusable for most users.
* **Problem 2: "Akin to burning" but not a direct burn:** While transferring tokens to a contract that doesn't intend to move them out can function as a "soft burn," it's not an explicit token burning mechanism. If the genuine intention is to remove tokens from circulation permanently, a dedicated `burn` function provided by the `i_snow` token (if available) or transferring to `address(0)` would be more explicit and gas-efficient.
* **Problem 3: Claiming vs. Staking/Locking/Swapping:** The function is named `claimSnowman`, and its final step, `i_snowman.mintSnowman(receiver, amount);`, suggests that the user *receives* `i_snowman` tokens. However, the `safeTransferFrom` step implies that the user is *giving up* their `i_snow` tokens. This creates a misleading user experience, as it's fundamentally a "swap" or "deposit-to-mint" operation rather than a straightforward "claim."

Risk

  1. Functionality Failure: The claimSnowman function will likely fail for the vast majority of users because they will not have pre-approved the SnowmanAirdrop contract to spend their i_snow tokens, rendering the core feature dysfunctional.

  2. Misleading User Experience: Users will expect to receive tokens when performing a "claim" operation, but they will instead be required to relinquish their existing i_snow tokens, leading to confusion and frustration.

  3. Security Risk (if approved): If a user does approve the contract, they might unknowingly be giving up their i_snow tokens without fully understanding the exchange, especially if the dApp's UI presents this as a simple "claim" rather than a "swap" or "deposit."


Proof of Concept

The problematic line is explicitly:
```javascript
// This line is problematic for a "claim" function
i_snow.safeTransferFrom(receiver, address(this), amount);
```

Recommended Mitigation

The primary recommendation is to clarify the intended functionality and re-architect the claimSnowman function accordingly to match the actual user flow.

  1. If it's a true "claim" (user receives tokens for free based on a condition, without giving up i_snow):
    * Remove the i_snow.safeTransferFrom line entirely.
    * The i_snowman.mintSnowman(receiver, amount); would then be the sole token transfer, where amount is determined by the i_snow.balanceOf(receiver) at the time of the snapshot (validated by Merkle proof) or a fixed claim amount. This allows users to claim i_snowman without surrendering their i_snow.

  2. If it's a "swap" or "deposit-to-mint" mechanism (user gives i_snow to receive i_snowman):
    * Rename the function to clearly reflect this interaction (e.g., swapSnowForSnowman, depositSnowForSnowman, stakeSnowForSnowman).
    * Clearly communicate to users that they need to approve the contract to spend their i_snow tokens before calling this function (this is a standard step for transferFrom).
    * In this scenario, the safeTransferFrom line would be functionally correct for the intended flow.

  3. If tokens are intended to be "burned":
    * If i_snow tokens are truly meant to be removed from circulation, use a dedicated burn function provided by the i_snow token (if it adheres to a standard like ERC20 Burnable) or transfer the tokens to a known burn address (e.g., address(0)) if i_snow is a custom token without a direct burn mechanism. This makes the intent explicit.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.