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

Default Status value is NICE, so anyone who was never checked can collect a present

niosoke

Root + Impact

The Status enum is ordered NICE, EXTRA_NICE, NAUGHTY, NOT_CHECKED_TWICE, so the zero value that every uninitialized mapping entry defaults to is NICE, not NOT_CHECKED_TWICE. Both list mappings therefore report NICE for every address that Santa has never looked at, and collectPresent() accepts exactly that combination. Any address, with no involvement from Santa at all, can mint itself a present NFT once the Christmas timestamp has passed (which it long has). The entire naughty-or-nice gating is bypassed by default.

Description

// @> enum Status {
// @> NICE, // 0 - the mapping default!
// @> EXTRA_NICE, // 1
// @> NAUGHTY, // 2
// @> NOT_CHECKED_TWICE // 3 - intended default, but never the default
// @> }
​
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.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
// @> _mintAndIncrement();
return;
}
...
}

For a never-checked address, getNaughtyOrNiceOnce and getNaughtyOrNiceTwice both return NICE (0), so the first branch of collectPresent succeeds. The NOT_CHECKED_TWICE member exists precisely to represent "not checked", but because it sits at index 3 it is never the value an unchecked address holds.

Risk

Likelihood: High - no preconditions beyond an ordinary wallet; the Christmas block time (25 Dec 2023) is in the past, so the exploit works right now, for everyone, repeatedly across fresh addresses (sybils are free).
Impact: High - the present NFT, the protocol's core asset that is supposed to be earned by being on Santa's nice list, can be minted by arbitrary unchecked addresses; the list loses all meaning.

Proof of Concept

Verified with Foundry (test passes): a fresh address that Santa never interacted with reads as NICE on both lists and successfully collects a present.

function test_AnyoneCanCollectWithoutBeingChecked() public {
assertEq(uint256(list.getNaughtyOrNiceOnce(user)), uint256(SantasList.Status.NICE));
assertEq(uint256(list.getNaughtyOrNiceTwice(user)), uint256(SantasList.Status.NICE));
vm.prank(user);
list.collectPresent();
assertEq(list.balanceOf(user), 1);
}

Recommended Mitigation

Make the unchecked state the zero value of the enum:

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

Lead Judging Commences

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