`checkTwice` Vulnerable to Default Enum Value Bypass
Root + Impact
Description
-
The `checkTwice` function doesn't verify that an address was actually checked once before being checked twice. In Solidity, uninitialized enum mappings return the first enum value (0 = `NICE`), allowing Santa to mark someone as checked twice with `NICE` status even if they were never checked once.
### Root + Impact
**Description:**
* The normal behavior is that Santa should only be able to check someone twice if they were first checked once with a matching status.
* The issue is that `checkTwice` only checks if the first check status matches the second check status, but doesn't verify that the address was actually checked once. Since uninitialized enum mappings return the default value (first enum = `NICE`), Santa can check someone twice with `NICE` status even if they were never checked once.
```solidity
// @> SantasList.sol:69-74
enum Status {
NICE, *// @> Default value (0) for uninitialized mappings*EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}
```
```solidity
// @> SantasList.sol:133-139
function checkTwice(address person, Status status) external onlySanta {
if (s_theListCheckedOnce[person] != status) { // @> For unchecked addresses, this is NICE (0) == NICE (0), so passes!
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}
```
Risk
Likelihood:
-
* This occurs when Santa tries to check someone twice who was never checked once with `NICE` status
* The default enum value behavior makes this bypass possible
Impact:
-
* Santa can mark people as checked twice without checking them once, bypassing the two-step verification process
* This undermines the security model where addresses must be checked twice to be eligible for presents
* Allows premature or incorrect status assignment
Proof of Concept
Recommended Mitigation
Lead Judging Commences
[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(); } ```
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.