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

All addresses are considered `NICE` by default and are able to claim a NFT through `collectPresent` function before any Santa check.

tadev

Summary

collectPresent function is supposed to be called by users that are considered NICE or EXTRA_NICE by Santa. This means Santa is supposed to call checkList function to assigned a user to a status, and then call checkTwice function to execute a double check of the status.

Currently, the enum Status assigns its default value (0) to NICE. This means that both mappings s_theListCheckedOnce and s_theListCheckedTwice consider every existent address as NICE. In other words, all users are by default double checked as NICE, and therefore eligible to call collectPresent function.

Vulnerability Details

The vulnerability arises due to the order of elements in the enum. If the first value is NICE, this means the enum value for each key in both mappings will be NICE, as it corresponds to 0 value.

Impact

The impact of this vulnerability is HIGH as it results in a flawed mechanism of the present distribution. Any unchecked address is currently able to call collectPresent function and mint an NFT. This is because this contract considers by default every address with a NICE status (or 0 value).

Proof of Concept

The following Foundry test will show that any user is able to call collectPresent function after CHRISTMAS_2023_BLOCK_TIME :

function testCollectPresentIsFlawed() external {
// prank an attacker's address
vm.startPrank(makeAddr("attacker"));
// set block.timestamp to CHRISTMAS_2023_BLOCK_TIME
vm.warp(1_703_480_381);
// collect present without any check from Santa
santasList.collectPresent();
vm.stopPrank();
}

Tools Used

Foundry

Recommendations

I suggest to modify Status enum, and use UNKNOWN status as the first one. This way, all users will default to UNKNOWN status, preventing the successful call to collectPresent before any check form Santa:

enum Status {
UNKNOWN,
NICE,
EXTRA_NICE,
NAUGHTY
}

After modifying the enum, you can run the following test and see that collectPresent call will revert if Santa didn't check the address and assigned its status to NICE or EXTRA_NICE :

function testCollectPresentIsFlawed() external {
// prank an attacker's address
vm.startPrank(makeAddr("attacker"));
// set block.timestamp to CHRISTMAS_2023_BLOCK_TIME
vm.warp(1_703_480_381);
// collect present without any check from Santa
vm.expectRevert(SantasList.SantasList__NotNice.selector);
santasList.collectPresent();
vm.stopPrank();
}
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.