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

Default enum value may lead to misleading initial status

ael404

Root + Impact

Description

The contract defines a Status enum where the first value (0) is NICE. A mapping from address to Status is used. In Solidity, any address not explicitly set in the mapping will return the default value of the enum, which is the first declared value (NICE in this case).

While this does not currently cause functional issues in the contract’s logic, it could be misleading when interpreting the initial state of addresses, as all addresses will appear as NICE by default even if they have not been processed yet.

enum Status {
@> NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

Risk

  • Misinterpretation of state: Users or interfaces may assume that addresses returning NICE have been explicitly processed, even if they have never interacted with the contract.

  • Future logic bugs: If new functions rely on distinguishing between processed and unprocessed addresses, the default value could lead to incorrect behavior.

  • Testing and audit inconsistencies: Without a distinct “unset” value, tests may overlook the difference between default and explicitly assigned statuses, creating a false sense of security.

Proof of Concept

This test demonstrates that an address not explicitly set in the s_theListCheckedOnce mapping returns the default enum value Status.NICE. It confirms that the contract’s mapping returns the first enum value for unset addresses, validating the behavior described in the audit note.

function test__statusEnum() public {
address userNotSetInTheMapping = makeAddr("userNotSet");
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(userNotSetInTheMapping)),
uint256(SantasList.Status.NICE)
);
}

Recommended Mitigation

Optionally add a UNSET or NOT_CHECKED enum value as the first entry in the enum to clearly distinguish addresses that have not yet been assigned a status.

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

Lead Judging Commences

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