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 over 1 year 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.