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

Users don't need to be checked (twice) to collect present

0x141414

Summary

Since the Status enum's default state is NICE, after the CHRISTMAS_2023_BLOCK_TIME has passed, any user may call the external function collectPresent() and mint an NFT for themselves without being checked twice by Santa address.

Vulnerability Details

The Status enum is structured in such that the default state is NICE, as shown below.

enum Status {
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

This exposes an issue within the external function collectPresent(). According to the documentation, "In order for someone to be considered NICE or EXTRA_NICE they must be first "checked twice" by Santa." Because the default state for every user for both mappings s_theListCheckedOnce[msg.sender] and s_theListCheckedTwice[msg.sender] equates to NICE, this allows the below check in collectPresent() to always be true by default, without the 2nd check from the santa wallet.

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

Here is a PoC written as a forge test demonstrating a user's ability to mint an NFT without being checked once or twice by Santa. This test passes in the forge workspace.

function test_uncheckedUserCanCollect() public {
address user = makeAddr("user");
SantasList.Status defaultEnumStatus = SantasList.Status.NICE;
​
// check initial `user` once status
SantasList.Status onceStatus = santasList.getNaughtyOrNiceOnce(user);
assert(onceStatus == defaultEnumStatus);
​
// check initial `user` twice status
SantasList.Status twiceStatus = santasList.getNaughtyOrNiceTwice(user);
assert(twiceStatus == defaultEnumStatus);
​
// Note santa never checks list for `user`
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.prank(user);
santasList.collectPresent();
​
assert(santasList.balanceOf(user) > 0);
}

Impact

Any user, may mint an NFT despite not being checked twice.

Tools Used

Forge

Recommendations

Set another non-permitting state as the Status enum's default (highest-level) state. Options may inclue UKNOWN or UNCHECKED. See example below.

enum Status {
UNKNOWN,
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
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.