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

Default Enum value check

raihanmd

Wrong Logic in SantasList::checkTwice() — Default Enum Value Allows Bypassing First Check Requirement

Description

SantasList::checkTwice() is intended to only be callable for addresses that have already been processed through checkList() first. The natspec clearly states:

"Do a second pass on someone if they are naughty or nice. Only if they pass this are they eligible for a present."

However, the validation logic in checkTwice() only checks whether the status in s_theListCheckedOnce matches the input status — it does not verify whether the address has ever been added to the first list at all:

function checkTwice(address person, Status status) external onlySanta {
// @audit wrong logic — only checks value equality, not existence
if (s_theListCheckedOnce[person] != status) {
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}

The root cause lies in Solidity's default storage behavior: all uninitialized mapping values default to 0, and since Status.NICE is the first member of the enum (index 0), any address that has never been passed through checkList() will have s_theListCheckedOnce[person] == Status.NICE by default.

enum Status {
NICE, // = 0 ← default value for all uninitialized addresses!
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

This means Santa can — intentionally or accidentally — call checkTwice(randomAddress, Status.NICE) for any address that was never checked once, and the call will succeed without reverting.

Risk

Likelihood: MEDIUM

  • Requires Santa to be the caller (access controlled)

  • However, Santa could be socially engineered, make an honest mistake, or act maliciously

  • Combined with the missing access control bug on checkList(), any attacker can first self-register via checkList() and then push Santa to confirm via checkTwice()

Impact: HIGH

  • Addresses that were never legitimately reviewed can be placed on the second list as NICE or EXTRA_NICE

  • These addresses become eligible to call collectPresent() and receive NFTs and SantaToken they were never entitled to

  • Breaks the core two-step verification invariant of the entire protocol

Proof of Concept

function test_defaultEnumBypass() public {
// randomUser has NEVER been passed through checkList()
// but s_theListCheckedOnce[randomUser] == Status.NICE (default = 0)
​
// Santa calls checkTwice with NICE — this should revert but doesn't
vm.prank(santa);
santasList.checkTwice(randomUser, SantasList.Status.NICE);
​
// randomUser is now on the second list as NICE
// They can now collect an NFT they were never entitled to
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.prank(randomUser);
santasList.collectPresent();
​
assertEq(santasList.balanceOf(randomUser), 1); // should never have reached here
}

Recommended Mitigation

Introduce a separate boolean mapping to explicitly track whether an address has been processed through checkList(), independent of the status value:

+ mapping(address person => bool) private s_hasBeenCheckedOnce;
​
function checkList(address person, Status status) external onlySanta {
+ s_hasBeenCheckedOnce[person] = true;
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
​
function checkTwice(address person, Status status) external onlySanta {
+ if (!s_hasBeenCheckedOnce[person]) {
+ revert SantasList__NotCheckedOnce();
+ }
if (s_theListCheckedOnce[person] != status) {
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}

Additionally, consider placing UNKNOWN or NOT_CHECKED_TWICE as the first member of the enum so the default value is a neutral, non-privileged state:

enum Status {
+ UNKNOWN, // = 0 ← safe default
NICE,
EXTRA_NICE,
NAUGHTY,
- NOT_CHECKED_TWICE
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

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

## Description `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(); } ``` ## 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(); } ```

Support

FAQs

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

Give us feedback!