First Flight #5: Santa's List
First Flight #5
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong check to prevent minting duplicate NFTs

dentonylifer

Summary

Wrong check in the codebase to prevent someone from minting duplicate NFTs.

Vulnerability Details

To prevent the user from receiving a gift twice, the "collectPresent" function has the following check:

if (balanceOf(msg.sender) > 0) { AlreadyHasPresent
revert SantasList__AlreadyCollected();
}

Malicious user can bypass this check by simply transferring the gift to his second address, so his current address balance will be 0.

Impact

a malicious user who passes two-step verification will be able to:

  1. call the "collectPresent" function and receive a gift;

  2. forward the gift to his second address;

  3. call the "collectPresent" function again and receive another gift;

  4. forward the gift to his second address again...;
    This cycle will continue indefinitely.

Tools Used

Manual review.

Recommendations

Instead of checking balanceOf, recommended to add a new mapping "hasCollectedPresent" and change the function "collectPresent":

mapping(address => bool) private hasCollectedPresent;
​
function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (hasCollectedPresent[msg.sender]) {
revert SantasList__AlreadyCollected();
}
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
hasCollectedPresent[msg.sender] = true;
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE && s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
hasCollectedPresent[msg.sender] = true;
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.