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

Enum structure allows anyone to collect a present when they are not in lists

MikeDougherty

Summary

Since Status.NICE is the first element in the SantasList::Status enumerator it is the equivalent to zero (0), thereby allowing any address not in the list to be considered Status::NICE.

Vulnerability Details

Solidity uses zero as defaults for a two things cause the SantasList::collectPresent() function to allow any address that has not been checked to collect a Status.NICE present. This is because each item in the SantasList::Status is compiled to uint value starting with 0, so Status.NICE == 0. Additionally, when a value is not present in a mapping the result will be zero (0) as well. So this conditional will be true for every address that is not in either mapping.

if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
}

This can be verified with by adding the following test case to SantasListTest.t.sol:

function testAnyoneCanCollectPresent() public {
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
uint256 stealCounter = 10;
for (uint256 index = 0; index < stealCounter; index++) {
address userNotInList = makeAddr(string(abi.encode("userNotInList-", index)));
vm.startPrank(userNotInList);
santasList.collectPresent();
assertEq(santasList.balanceOf(userNotInList), 1, "user did not collect 1 NFT");
assertEq(santasList.ownerOf(index), userNotInList, "user does not own @index NFT");
}
}

Impact

Anyone can create as many addresses as they want and collect as many NFTs as they want.

Tools Used

Manual Review and Foundry

Recommendations

Reordering the elements in the SantasList::Status so that Status::NOT_CHECKED_TWICE is the first item and therefore is zero (0):

enum Status {
+ NOT_CHECKED_TWICE,
NICE,
EXTRA_NICE,
- NAUGHTY,
- NOT_CHECKED_TWICE
+ NAUGHTY
}

Then testing addresses that have not been checked yet will become the same as if they had been added to both lists with the status of Status::NOT_CHECKED_TWICE.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

default status is nice

In Solidity the first element of an enum is the default value. In Santa's List, the means each person is mapped by default to 'NICE'.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!