Santa's List

AI First Flight #3
Beginner FriendlyFoundry
EXP
View results
Submission Details
Severity: high
Valid

[H-1] Unchecked Addresses Can Claim Presents Due to Enum Default Value

Unchecked Address Can Claim Present + Unauthorized Mint

Description

  • The intended behavior is that only addresses that have been checked twice and received a qualifying Status (e.g., NICE or EXTRA_NICE) can successfully call collectPresent() and mint a present (and optionally additional rewards).

  • The contract stores check results in mapping(address => Status). For mappings, any key that has never been written returns the default value of the mapped type. For enums, the default value is the first enum variant, i.e., underlying value 0. Since Status.NICE is currently the first enum value (0), an address that has never been checked will read as Status.NICE in both s_theListCheckedOnce[msg.sender] and s_theListCheckedTwice[msg.sender]. As a result, the first branch in collectPresent() can evaluate to true for an unchecked address, allowing it to mint a present.

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
// Root cause: mappings return default enum value (0) for unset keys.
// If Status.NICE == 0, unchecked addresses appear as NICE in both mappings.
if (
s_theListCheckedOnce[msg.sender] == Status.NICE && // @> default enum value can satisfy this
s_theListCheckedTwice[msg.sender] == Status.NICE // @> default enum value can satisfy this
) {
_mintAndIncrement(); // @> unauthorized mint
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE &&
s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}

Risk

Likelihood:

  • After CHRISTMAS_2023_BLOCK_TIME, any address that has never been checked has s_theListCheckedOnce[address] and s_theListCheckedTwice[address] both resolving to the enum default value

  • When the enum default value corresponds to a “valid/allow” status (Status.NICE), the first eligibility branch becomes reachable without any prior checks

Impact:

  • Any unchecked address can mint a present, violating the core authorization logic of the system

  • Unbounded unauthorized minting can inflate supply, break token economics, and undermine trust in the “nice list” gating mechanism (and may additionally mint external rewards in related branches/designs)

Proof of Concept

function testUncheckedAddressCanCollectPresent() public {
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.startPrank(user);
santasList.collectPresent();
assertGt(santasList.balanceOf(user), 0);
}

Recommended Mitigation

Introduce an explicit sentinel enum value representing an unchecked state as the first enum variant (so it becomes the default 0), and require that the sender has been checked before allowing present collection.

- enum Status {
- NICE,
- EXTRA_NICE,
- NAUGHTY,
- NOT_CHECKED_TWICE
- }
+ enum Status {
+ NOT_CHECKED, // default value (0) for unset mapping keys
+ NICE,
+ EXTRA_NICE,
+ NAUGHTY,
+ NOT_CHECKED_TWICE
+ }

Then update collectPresent() to ensure the caller has been checked:

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
+ if (s_theListCheckedOnce[msg.sender] == Status.NOT_CHECKED) {
+ revert SantasList__NotChecked();
+ }
if (
s_theListCheckedOnce[msg.sender] == Status.NICE &&
s_theListCheckedTwice[msg.sender] == Status.NICE
) {
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE &&
s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}

Note: Simply reordering the enum so NAUGHTY is first avoids the specific “unchecked == NICE” bypass, but it still conflates “unchecked” with a real business status and can create other unintended behaviors. A dedicated NOT_CHECKED sentinel (or an explicit checkedOnce/checkedTwice boolean) is the most robust fix.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 3 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!