Santa's List

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

NICE is enum member 0, so every unwritten mapping slot is already a passing verdict — any address mints a present with no grading

## [H-1] `NICE` is enum member 0, so every unwritten mapping slot is already a passing verdict — any address mints a present with no grading

**Impact:** High · **Likelihood:** High

### Description

- A present is meant to be earned. The specification states that *"in order for someone to be

considered `NICE` or `EXTRA_NICE` they **must** be first 'checked twice' by Santa"*, and

`collectPresent` reads both status mappings to enforce it.

- `Status` declares `NICE` first, so `NICE == 0`, and `0` is what every unwritten mapping slot

returns. Both mappings already read `NICE` for every address in existence, which is exactly the

branch condition at `:154`. Santa's grading confers nothing the default does not already confer.

```solidity

// src/SantasList.sol:69-74

enum Status {

@> NICE, // member 0 — the default value of every unwritten slot

EXTRA_NICE,

NAUGHTY,

NOT_CHECKED_TWICE

}

// src/SantasList.sol:79-80

mapping(address person => Status naughtyOrNice) private s_theListCheckedOnce;

mapping(address person => Status naughtyOrNice) private s_theListCheckedTwice;

// src/SantasList.sol:154

@> if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {

_mintAndIncrement();

return;

}

```

### Risk

**Likelihood:**

- Every address on the chain satisfies the branch at `:154` from the moment the contract is

deployed, with no transaction sent and no state written.

- `collectPresent` is `external` with no modifier, and the only other gate — `block.timestamp >=

CHRISTMAS_2023_BLOCK_TIME` — passed on 2023-12-25.

**Impact:**

- Presents are minted from unlimited fresh addresses for gas alone. The PoC mints 100 against 2

legitimately graded users.

- The issuance is irreversible: there is no burn function, no cap, no pause, no owner, and no

upgrade path. `i_santa` and `i_santaToken` are `immutable` with no setters.

  • Bounded: this path yields the NFT and **no `SantaToken`**. `EXTRA_NICE` is member 1, not a

default, and is writable only through the `onlySanta`-gated `checkTwice`.

### Proof of Concept

```solidity

function test_F01_freeMint() public {

address freshAttacker = makeAddr("freshAttacker"); // never graded, never seen by Santa

vm.warp(1_703_480_381); // Christmas 2023

assertEq(uint256(list.getNaughtyOrNiceOnce(freshAttacker)), uint256(SantasList.Status.NICE));

assertEq(uint256(list.getNaughtyOrNiceTwice(freshAttacker)), uint256(SantasList.Status.NICE));

vm.prank(freshAttacker);

list.collectPresent();                       *// no checkList, no privilege, no capital*

assertEq(list.balanceOf(freshAttacker), 1);

}

```


### Recommended Mitigation

```diff

- enum Status { NICE, EXTRA_NICE, NAUGHTY, NOT_CHECKED_TWICE }

+ enum Status { NOT_CHECKED_TWICE, NICE, EXTRA_NICE, NAUGHTY }

```

Any equivalent works — gate on `s_theListCheckedTwice[msg.sender] != Status.NOT_CHECKED_TWICE`, or

add an explicit `mapping(address => bool) s_graded`. A verdict must not be the default value of an

unwritten slot.

Updates

Lead Judging Commences

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