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

Unchecked users can collect presents because the default enum value is `NICE`

fyuryy

Description

  • The `collectPresent()` function is supposed to allow only users who have been checked by Santa and confirmed as `NICE` or `EXTRA_NICE` in both checks to collect a present.

  • However the Statusenum defines NICE as its first value:

  • In Solidity, the default value of an enum is its first value. Since mappings return the default value for keys that were never written to, any address that was never checked by Santa will have both checklist mappings default to Status.NICE. As a result, an unchecked user passes the NICE && NICEcondition in collectPresent()and can mint themselves an NFT present.

enum Status {
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
​
function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
​
// @> Unchecked users pass this condition because both mappings default to Status.NICE
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
​
revert SantasList__NotNice();
}

This breaks the intented eligibility mechanism because users who were never checked by Santa are treated as nice by default

Risk

Likelihood:

  • This occurs whenever an address that was never checked calls `collectPresent()` after Christmas.

  • Solidity mappings return the default ENUM value for unset keys, and Status.NICEis the default value because it's declared first.

Impact:

  • Any unchecked address can collect an NFT present.

  • Santa's checklist process is bypassed.

Proof of Concept

Here, an attacker that has never been checked can mint an NFT for themselves.

function testUncheckedUserCanCollectPresent() public {
address attacker = makeAddr("attacker");
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.prank(attacker);
santasList.collectPresent();
​
assertEq(santasList.balanceOf(attacker), 1);
assertEq(santasList.getNaughtyOrNiceOnce(attacker), SantasList.Status.NICE);
assertEq(

Recommended Mitigation

Add an explicit unchecked status as the first enum value so the default value does not represent an eligible status.

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

Lead Judging Commences

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