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

Users can claim rewards even if they were not checked

Summary

In the Santa's List protocol, users can claim rewards if they were checked twice by Santa and determined to be NICE or EXTRA_NICE. Two mappings are used to determine that - s_theListCheckedOnce and s_theListCheckedTwice. The problem is that the default value for any address in those mappings is NICE. Therefore anyone can claim their rewards before they are checked by Santa.

Vulnerability Details

The SantasList::collectPresent() method contains the following check:

if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
}

The sender's address is used to look up the status in two mappings - s_theListCheckedOnce and s_theListCheckedTwice, which are defined as follows:

mapping(address person => Status naughtyOrNice) private s_theListCheckedOnce;
mapping(address person => Status naughtyOrNice) private s_theListCheckedTwice;

The issue lays in the definition of enum Status:

enum Status {
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

When the two mappings are initialized, the value for each address is the default value for enum - the first value in enum's definition. As one can see in the enum's definition, the default value for Status is NICE.

That means that any address by default is already "checked twice" and has the status of NICE. Even if a user has not been checked by Santa, by default their are eligible for claiming the NFT.

Impact

Users can claim NFT even if they were not checked by Santa.

Tools Used

Manual review.

Recommendations

Make the NOT_CHECKED_TWICE the default value for enum Status.

enum Status {
- NICE,
+ NOT_CHECKED_TWICE,
EXTRA_NICE,
NAUGHTY,
- NOT_CHECKED_TWICE
+ NICE
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

default status is nice

In Solidity the first element of an enum is the default value. In Santa's List, the means each person is mapped by default to 'NICE'.

Support

FAQs

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