Santa's List

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

Status enum defaults to NICE, so any address Santa never checked mints a free present via collectPresent

Status.NICE is the enum's zero value, so every address Santa never checked is treated as NICE and can mint a free present through collectPresent.

Description

  • A present should only be collectable by an address that Santa explicitly marked NICE or EXTRA_NICE on both the first and second pass.

  • NICE is declared first in the Status enum (src/SantasList.sol:69), so it equals 0, which is the default value of every unset mapping entry. Both s_theListCheckedOnce and s_theListCheckedTwice therefore read back as NICE for any address Santa never touched, and the NICE branch of collectPresent (src/SantasList.sol:154) passes for everyone.

// src/SantasList.sol
enum Status {
@> NICE, // 0 = default value of any unset address
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
// ...
function collectPresent() external {
// ...
@> if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
}
// ...
}

Risk

Likelihood:

  • Any externally owned account can call collectPresent after the Christmas timestamp with no setup and mint a present.

  • The only other gates are the timestamp and balanceOf(msg.sender) == 0, both of which a fresh address satisfies.

Impact:

  • The two-pass naughty-or-nice review is bypassed entirely for the base NICE reward, so anyone mints a present NFT for free.

  • The list provides no access control over the core reward. This is a High severity issue.

Proof of Concept

This test mints a present to a ghost address Santa never checked. It passes.

function test_F2_defaultNice_mintGratis() public {
address ghost = makeAddr("ghost"); // never touched by Santa
assertEq(uint256(santasList.getNaughtyOrNiceOnce(ghost)), uint256(SantasList.Status.NICE)); // default 0
assertEq(uint256(santasList.getNaughtyOrNiceTwice(ghost)), uint256(SantasList.Status.NICE));
assertEq(santasList.balanceOf(ghost), 0);
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.prank(ghost);
santasList.collectPresent();
// free NFT without ever being on the list
assertEq(santasList.balanceOf(ghost), 1);
}

Recommended Mitigation

Make the enum's zero value a non-eligible sentinel so an unchecked address never matches a reward branch. Reorder the enum so index 0 is a state no reward path accepts.

enum Status {
- NICE,
+ NOT_CHECKED_TWICE, // 0 = default for any unset address, never eligible
EXTRA_NICE,
- NAUGHTY,
- NOT_CHECKED_TWICE
+ NAUGHTY,
+ NICE
}

With index 0 mapped to NOT_CHECKED_TWICE, both mappings default to a non-eligible status, so the NICE and EXTRA_NICE branches fail for any address Santa did not explicitly approve and collectPresent reverts with SantasList__NotNice.

Updates

Lead Judging Commences

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