Normal: Santa Claus calls the checkList() and checkTwice() functions to modify the status value of the specified address.
Error: The default enumeration value is NICE, and the mapping function returns 0 by default, which corresponds to Status.NICE.Any address that has never been checked by Santa can directly claim an NFT after Christmas 2023, as long as the current NFT balance is 0.
Likelihood:
As long as the address is not designated by Santa Claus, their corresponding status value is defaulted to NICE.
Impact:
Once the designated time in the contract is reached, any address not specified by Santa Claus can directly call collectPresent() to claim the reward corresponding to NICE.
Once the designated time in the contract is reached, any address not specified by Santa Claus can directly call collectPresent() to claim the reward corresponding to NICE.
Place NOT_CHECKED_TWICE at the first position in the enumeration
## 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(); } ```
The contest is live. Earn rewards by submitting a finding.
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsThe contest is complete and the rewards are being distributed.