Santa's List

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

Default `Status` enum value is NICE(0), so any never-checked address can `collectPresent` and mint a present

Root + Impact

Description

  • Describe the normal behavior in one or more sentences

  • Explain the specific issue or problem in one or more sentences

## Description
​
The `Status` enum lists `NICE` first, so its default value (the value every address has before being checked) is `NICE` (0). `collectPresent` lets anyone whose once- and twice-status are both `NICE` collect a present — which is satisfied by **every address that was never checked at all**:
​
```solidity
enum Status { NICE, EXTRA_NICE, NAUGHTY, NOT_CHECKED_TWICE } // @> NICE == 0 == default
​
function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) revert ...;
if (balanceOf(msg.sender) > 0) revert ...;
if (s_theListCheckedOnce[msg.sender] == Status.NICE
&& s_theListCheckedTwice[msg.sender] == Status.NICE) { // @> both default to NICE(0)
_mintAndIncrement(); // unchecked address still mints a present
return;
}
...
}
```
​
Because both `s_theListCheckedOnce` and `s_theListCheckedTwice` return `Status.NICE` (0) for any address Santa never touched, an arbitrary address that was never put on the list passes the NICE/NICE branch and collects a present. Santa's naughty/nice gating is void: being "nice" is the default, not something Santa must grant.
​
## Risk
​
**Likelihood: High**
​
- Every address is NICE by default; after the Christmas timestamp, anyone who never interacted with Santa can call `collectPresent` and succeed.
​
**Impact: High**
​
- The core authorization of the protocol is broken: presents (NFTs) are mintable by anyone, not only addresses Santa marked nice. This defeats the naughty/nice list entirely and allows unlimited unauthorized minting (one per fresh address).
​
## Proof of Concept
​
```solidity
// Never call checkList/checkTwice for `attacker`.
// vm.warp(CHRISTMAS_2023_BLOCK_TIME);
// vm.prank(attacker);
// santasList.collectPresent(); // succeeds: both statuses default to NICE -> mints an NFT
```
​
## Recommended Mitigation
​
Use a non-collectable default. Reorder the enum so the zero value is a sentinel (e.g. `NOT_CHECKED_TWICE` / `NAUGHTY` first), or require an explicit positive check rather than equality to the default:
​
```solidity
enum Status { NOT_CHECKED_TWICE, NICE, EXTRA_NICE, NAUGHTY } // default is now a non-nice sentinel
```
}

Risk

Likelihood:

  • Reason 1 // Describe WHEN this will occur (avoid using "if" statements)

  • Reason 2

Impact:

  • Impact 1

  • Impact 2

Proof of Concept

​

Recommended Mitigation

- remove this code
+ add this code
Updates

Lead Judging Commences

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