Santa's List

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

Anyone can collect a present without Santa ever listing them, because an uninitialized status reads as NICE — the naughty/nice gate is bypassed entirely

Default-NICE status makes the naughty/nice allow-list a no-op, bypassing all access control on present collection

Description

Santa's checkList/checkTwice exist to gate who may collect a present, but because the Status enum defaults to NICE (index 0), no address ever needs to be checked. The gate in collectPresent is satisfied by the default mapping value, so the allow-list mechanism enforces nothing.

// SantasList.sol:154
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement(); // @> reached by anyone: both sides default to NICE
return;

Risk

Likelihood: High

The access gate relies entirely on per-address status that Santa sets. Unset entries read as NICE, so the "only nice people may collect" rule is automatically true for the whole address space without Santa's involvement.

Impact: High

The intended access control is fully bypassed. Anyone may collect a present regardless of whether Santa ever evaluated them, making checkList and checkTwice decorative rather than authoritative and breaking the core invariant of the contract.

Proof of Concept

An address Santa explicitly never approved still passes the gate and mints.

function test_listMechanismProvidesNoGate() public {
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME());
address notApproved = makeAddr("notApproved");
// Santa never calls checkList/checkTwice for notApproved
vm.prank(notApproved);
santasList.collectPresent();
assertEq(santasList.balanceOf(notApproved), 1);
}

Recommended Mitigation

Require an explicit non-default checked state before granting a present.

function collectPresent() external {
...
+ if (s_theListCheckedTwice[msg.sender] == Status.NOT_CHECKED_TWICE) {
+ revert SantasList__NotNice();
+ }
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
Updates

Lead Judging Commences

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