Snowman Merkle Airdrop

First Flight #42
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: low
Valid

Updates "hasAlreadyClaimed" to true, but doesn't check if receiver has already claimed

Root + Impact

Description

  • Actual behavior => the aidropSnowman.sol contract updates s_hasClaimedSnowman[receiver] to true after claiming. But never checks if s_hasClaimedSnowman[receiver] == false before claiming. s_hasClaimedSnowman[receiver] is then useless.

function claimSnowman(address receiver, bytes32[] calldata merkleProof, uint8 v, bytes32 r, bytes32 s)
external
nonReentrant
{
if (receiver == address(0)) {
revert SA__ZeroAddress();
}
if (i_snow.balanceOf(receiver) == 0) {
revert SA__ZeroAmount();
}
if (!_isValidSignature(receiver, getMessageHash(receiver), v, r, s)) {
revert SA__InvalidSignature();
}
// @> No check for s_hasClaimedSnowman[receiver] == false
uint256 amount = i_snow.balanceOf(receiver);
bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(receiver, amount))));
if (!MerkleProof.verify(merkleProof, i_merkleRoot, leaf)) {
revert SA__InvalidProof();
}
i_snow.safeTransferFrom(receiver, address(this), amount); // send tokens to contract... akin to burning
// Setting s_hasClaimedSnowman[receiver] = true is useless unless you check for it before claiming
@> s_hasClaimedSnowman[receiver] = true;
emit SnowmanClaimedSuccessfully(receiver, amount);
i_snowman.mintSnowman(receiver, amount);
}

Risk

Likelihood: High

  • It occurs everytime the function is called. No matter if s_hasClaimedSnowman[receiver] == true or false.

Impact: Low

  • If s_hasClaimedSnowman[receiver] is set to true after claiming it means the normal behavior should be to not allow to claim again if already claimed. Then it allows to bypass the intended restriction.

  • But I consider it LOW because the user can not claim again with the same tokens. He has to use what's left or buy some more. So it doesn't really affect the protocol beside the fact that it bypasses the intended restriction / normal behavior.

Proof of Concept

=> Try to claim multiple times for the same receiver :

1/ Buy some SNOW tokens.

2/ Claim a first time

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.24;
import {Test, console2} from "forge-std/Test.sol";
import {Snow} from "../src/Snow.sol";
import {Snowman} from "../src/Snowman.sol";
import {SnowmanAirdrop} from "../src/SnowmanAirdrop.sol";
import {MockWETH} from "../src/mock/MockWETH.sol";
import {Helper} from "../script/Helper.s.sol";
contract TestSnowmanAirdrop is Test {
Snow snow;
Snowman nft;
SnowmanAirdrop airdrop;
MockWETH weth;
Helper deployer;
bytes32 public ROOT = 0xc0b6787abae0a5066bc2d09eaec944c58119dc18be796e93de5b2bf9f80ea79a;
// Proofs
bytes32 alProofA = 0xf99782cec890699d4947528f9884acaca174602bb028a66d0870534acf241c52;
bytes32 alProofB = 0xbc5a8a0aad4a65155abf53bb707aa6d66b11b220ecb672f7832c05613dba82af;
bytes32 alProofC = 0x971653456742d62534a5d7594745c292dda6a75c69c43a6a6249523f26e0cac1;
bytes32[] AL_PROOF = [alProofA, alProofB, alProofC];
// Multi claimers and key
address alice;
uint256 alKey;
address satoshi;
function setUp() public {
deployer = new Helper();
(airdrop, snow, nft, weth) = deployer.run();
(alice, alKey) = makeAddrAndKey("alice");
satoshi = makeAddr("gas_payer");
}
function testClaimSnowman() public {
// Alice claim test
assert(nft.balanceOf(alice) == 0);
vm.prank(alice);
snow.approve(address(airdrop), 1);
// Get alice's digest
bytes32 alDigest = airdrop.getMessageHash(alice);
// alice signs a message
(uint8 alV, bytes32 alR, bytes32 alS) = vm.sign(alKey, alDigest);
// satoshi calls claims on behalf of alice using her signed message
vm.prank(satoshi);
airdrop.claimSnowman(alice, AL_PROOF, alV, alR, alS);
assert(nft.balanceOf(alice) == 1);
assert(nft.ownerOf(0) == alice);
}
}

3/ Buy more SNOW tokens.

4/ Claim a 2nd time

5/ See that it goes through.

Recommended Mitigation

Add a check on s_hasClaimedSnowman[receiver] :

function claimSnowman(address receiver, bytes32[] calldata merkleProof, uint8 v, bytes32 r, bytes32 s)
external
nonReentrant
{
if (receiver == address(0)) {
revert SA__ZeroAddress();
}
if (i_snow.balanceOf(receiver) == 0) {
revert SA__ZeroAmount();
}
if (!_isValidSignature(receiver, getMessageHash(receiver), v, r, s)) {
revert SA__InvalidSignature();
}
+ require(s_hasClaimedSnowman[receiver] != true);
uint256 amount = i_snow.balanceOf(receiver);
bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(receiver, amount))));
if (!MerkleProof.verify(merkleProof, i_merkleRoot, leaf)) {
revert SA__InvalidProof();
}
i_snow.safeTransferFrom(receiver, address(this), amount);
s_hasClaimedSnowman[receiver] = true;
emit SnowmanClaimedSuccessfully(receiver, amount);
i_snowman.mintSnowman(receiver, amount);
}

Or just remove the update of s_hasClaimedSnowman[receiver] to true each time there is a claiming because it's not used anywhere else :

function claimSnowman(address receiver, bytes32[] calldata merkleProof, uint8 v, bytes32 r, bytes32 s)
external
nonReentrant
{
if (receiver == address(0)) {
revert SA__ZeroAddress();
}
if (i_snow.balanceOf(receiver) == 0) {
revert SA__ZeroAmount();
}
if (!_isValidSignature(receiver, getMessageHash(receiver), v, r, s)) {
revert SA__InvalidSignature();
}
uint256 amount = i_snow.balanceOf(receiver);
bytes32 leaf = keccak256(bytes.concat(keccak256(abi.encode(receiver, amount))));
if (!MerkleProof.verify(merkleProof, i_merkleRoot, leaf)) {
revert SA__InvalidProof();
}
i_snow.safeTransferFrom(receiver, address(this), amount);
- s_hasClaimedSnowman[receiver] = true;
emit SnowmanClaimedSuccessfully(receiver, amount);
i_snowman.mintSnowman(receiver, amount);
}
Updates

Lead Judging Commences

yeahchibyke Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Lack of claim check

The claim function of the Snowman Airdrop contract doesn't check that a recipient has already claimed a Snowman. This poses no significant risk as is as farming period must have been long concluded before snapshot, creation of merkle script, and finally claiming.

Support

FAQs

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