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

Wrong default value of `Status` data type allows anyone to collect NFT

Summary

Wrong default value of Status data type allows anyone to collect NFT.

Vulnerability Details

enum Status {
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
mapping(address person => Status naughtyOrNice)
private s_theListCheckedOnce;
mapping(address person => Status naughtyOrNice)
private s_theListCheckedTwice;

The default value of Status enum is NICE. Due to this, all the addresses in both the s_theListCheckedOnce and s_theListCheckedTwice variables will have NICE Status.

With NICE Status, any user/address can call collectPresent function without calling checkList and checkTwice functions and collect their present.

Impact

Due to the default NICE Status, any user can collect their NFT without any confirmation from Santa.

Hence, destroying the whole contract's purpose.

To execute this test : forge test -vvvvv

address attacker = makeAddr("attacker");
function test() public {
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.prank(attacker);
santasList.collectPresent();
assertEq(santasList.balanceOf(attacker), 1);
}

Tools Used

Manual Review

Recommendations

To mitigate the vulnerability, the order in which members are present in the Status enum should be changed, such that, the default value is neither NICE nor EXTRA_NICE.

Here's how you can modify it:

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

Lead Judging Commences

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