Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Users can collect multiple presents from 'SantasList.sol::collectPresents' if they transfer the token to a different address first

Summary

Users that are NICE or EXTRA_NICE can collect multiple presents from the 'SantasList.sol::collectPresents' function if they transfer the token to a different address and claim it again.

Vulnerability Details

Because the only check in the collectPresent function from stopping a NICE or EXTRA_NICE user from claiming multiple tokens is the balanceOf check; users can transfer the token to a different address and then claim it again.

Impact

The below test passes as true showing that a user can mint multiple tokens.

function testCanCollectMultiplePresentIfTransfer() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.startPrank(user);
santasList.collectPresent();
santasList.transferFrom(user, user2, 0);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
assertEq(santasList.balanceOf(user2), 1);
}

Tools Used

--Foundry

Recommendations

It is recommended to create a mapping of the users to track if they have already minted a token or not.

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
+ if (balanceOf(msg.sender) > 0 || checkMintedNFT[msg.sender] == true) {
revert SantasList__AlreadyCollected();
}
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
+ checkMintedNFT[msg.sender] = true;
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
+ checkMintedNFT[msg.sender] = true;
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Weak Already Collected Check

Relying on balanceOf > 0 in collectPresent() allows the msg.sender to send their present to another address and then collect again.

Support

FAQs

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

Give us feedback!