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

NICE at index 0 causes all uninitialized addresses to pass second verification by default

palsudhanshuxcodes

Root + Impact-

Status enum defines NICE at position 0. In Solidity, mapping default value is 0. So s_theListCheckedTwice[anyAddress] defaults to NICE even before Santa calls checkTwice. This breaks two-pass verification system.

The SantasList uses two-pass: checkList() first pass, checkTwice() second pass. Only addresses passing both should collect.

Root Cause

File: SantasList.sol - enum definition

enum Status {
NICE, // 0 - default for all addresses
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

Both mappings:
mapping(address => Status) s_theListCheckedOnce
mapping(address => Status) s_theListCheckedTwice

Default to 0 = NICE for any address that was never checked. In collectPresent():

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

Second condition is always true by default. No second pass needed.

Impact

Likelihood: High - No attacker action required for vulnerable state
Impact: Medium - Breaks core invariant of dual confirmation

Proof of Concept

function test_DefaultBypass() public {
SantasList list = new SantasList(santa);
// Only first check done by Santa
vm.prank(santa);
list.checkList(alice, SantasList.Status.NICE);
// Alice collects without second pass - PASSES!
vm.prank(alice);
list.collectPresent();
assertEq(list.balanceOf(alice), 1);
}

Description-Description
Status enum defines NICE at position 0. In Solidity, mapping default value is 0. So s_theListCheckedTwice[anyAddress] defaults to NICE even before Santa calls checkTwice. This breaks two-pass verification system.

## Summary
Status enum defines NICE at position 0. In Solidity, mapping default value is 0. So `s_theListCheckedTwice[anyAddress]` defaults to NICE even before Santa calls checkTwice. This breaks two-pass verification system.
​
## Vulnerability Details
File: SantasList.sol - enum definition
​
enum Status {
NICE, // @> 0
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
​
- s_theListCheckedOnce defaults to 0 = NICE
- s_theListCheckedTwice defaults to 0 = NICE
​
In collectPresent():
```solidity
if (s_theListCheckedOnce[msg.sender] == Status.NICE
&& s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
}

Risk

Likelihood:Affects every address from deployment

Impact:Two-pass design is broken. Intended security requirement of double confirmation from Santa does not hold. Any address that passes first check can collect without second check.

Proof of Concept-Description

Status enum defines NICE at position 0. In Solidity, mapping default value is 0. So s_theListCheckedTwice[anyAddress] defaults to NICE even before Santa calls checkTwice. This breaks two-pass verification system.

The SantasList uses two-pass: checkList() first pass, checkTwice() second pass. Only addresses passing both should collect.

Root Cause

File: SantasList.sol - enum definition

function test_DefaultBypass() public {
SantasList list = new SantasList(santa);
// No checkTwice called ever
vm.prank(santa);
list.checkList(alice, SantasList.Status.NICE);
// Alice collects without second pass
vm.prank(alice);
list.collectPresent(); // PASSES! because s_theListCheckedTwice[alice] == 0 == NICE by default
assertEq(list.balanceOf(alice), 1);
}

Recommended Mitigation-Put NOT_CHECKED or UNKNOWN at index 0:

## Description
SantasList uses a two-pass check: checkList (first) and checkTwice (second). The Status enum defines NICE at index 0. In Solidity, all mappings default to 0. Therefore s_theListCheckedTwice for any address that was never checked returns NICE by default, making the second verification pass without Santa ever calling checkTwice.
​
## Vulnerability Details
File: SantasList.sol - enum Status
​
enum Status {
NICE, // 0 - default value
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
​
When a user never checked twice:
s_theListCheckedTwice[user] == 0 == NICE (true by default)
​
This breaks the invariant in collectPresent():
`if (s_theListCheckedOnce[msg.sender] == NICE && s_theListCheckedTwice[msg.sender] == NICE)`
​
The second condition is true from deployment for all addresses.
​
## Impact
High Likelihood, Medium Impact: Any address that gets only first check can collect present without second Santa approval. With unguarded checkList, attacker can self-mark first pass and collect.
​
## Proof of Concept
// Only first check done
vm.prank(santa); list.checkList(alice, Status.NICE);
// Alice collects without ever calling checkTwice - succeeds due to default
vm.prank(alice); list.collectPresent();
assertEq(list.balanceOf(alice), 1);
​
## Mitigation
Root cause is enum order. NICE should not be default. The fix requires reordering enum so default (0) is a non-privileged state.
​
Recommended Mitigation:
Reorder enum to put NOT_CHECKED_TWICE at index 0. This ensures uninitialized mappings are considered NOT_CHECKED, not NICE.
​
```diff
enum Status {
+ NOT_CHECKED_TWICE,
NICE,
EXTRA_NICE,
- NAUGHTY,
- NOT_CHECKED_TWICE
+ NAUGHTY
}
Updates

Lead Judging Commences

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