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

Wrong use of `balanceOf` allows users to collect more than 1 NFT

0x0bserver

Summary

Wrong use of balanceOf allows users to collect more than 1 NFT.

Vulnerability Details

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
@> if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}

The logic used to determine whether the user can already collect NFT or not is wrong as the user can transfer the NFT he/she collected to his/her another address, making the balance of the current user's address's NFT 0 again.

Hence, satisfying the (balanceOf(msg.sender) > 0) condition again.

Impact

Due to the flawed logic, user can repeat the same behavior of first collecting NFT and then transferring it to his/her another address again and again.

Hence, collecting countless NFTs and defeating the major purpose of this function and the whole contract.

Tools Used

Manual Review

Recommendations

To mitigate the vulnerability, create a mapping(address => bool) that tracks whether user has collected the NFT or not.

Here's how you can modify the function:

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