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

Buying present for someone else prevents them from getting their own present

dianivanov

Summary

If I buy a present for someone else, via buyPresent(suppose it was really buying the presents for the receiver)
this check will prevent them from taking their own present from santa.

Vulnerability Details

For example Bob was an EXTRA_NICE user, collects his own present via "collectPresent" and buys a present for Alice, but Alice was also an EXTRA_NICE user and she hasn't collected her gift, but now her NFT balance is > 0 becauase Bob bought her a present resulting in Alice having to transfer this NFT to someone else to collect her own present:

// here alice's call to collectPresent will revert with SantasList__AlreadyCollected() even though she've never collected it
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}

Impact

Users can not collect their own present from santa.

Tools Used

Manual inspection

Recommendations

Add a mapping where you can store the addresses which has collected presents:

mapping(address person => bool) private s_personCollectedPresent;

Definitely remove the balance check and make following changes when a present is collected via "collectPresent":

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
​
​
// replace the previous balance checks with this one
if (s_personCollectedPresent[msg.sender] == true) {
revert SantasList__AlreadyCollected();
}
s_personCollectedPresent[msg.sender] = false;
​
​
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge
almost 2 years ago
inallhonesty Lead Judge almost 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.