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

wrongly handling the default value of `SantasList::Status` enum will cause a cascade of errors.

0xspryon

Summary

The application fails to properly account for the default value of SantasList::Status enum in SantasList::collectPresent, SantasList::checkTwice, SantasList::getNaughtyOrNiceTwice and SantasList::getNaughtyOrNiceOnce.

Vulnerability Details

The default value of an enum is it's first member
Thus, for the case of SantasList::Status enum, it's default value is NICE.
This leads:

  • Both SantasList::getNaughtyOrNiceOnce and SantasList::getNaughtyOrNiceTwice to return NICE for persons not present in SantasList::s_theListCheckedOnce and SantasList::s_theListCheckedTwice respectively

  • The first call to SantasList::collectPresent at past christmass 2023 block time by any caller that has not been SantasList::checkList-ed and SantasList::checkTwice-d to mint a token instead of reverting

POC

Put the following code excerpt in `test/unit/SantasListTest.tsol`
function testCollectPresentNotChecked() public {
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
address inexistentUser = makeAddr("inexistent_user");
​
vm.startPrank(inexistentUser);
santasList.collectPresent();
assertEq(santasList.balanceOf(inexistentUser), 1);
vm.stopPrank();
}
​
function testCheckListInexistentUser() public {
address inexistentUser = makeAddr("inexistent_user");
assertEq(uint256(santasList.getNaughtyOrNiceOnce(inexistentUser)), uint256(SantasList.Status.NICE));
assertEq(uint256(santasList.getNaughtyOrNiceTwice(inexistentUser)), uint256(SantasList.Status.NICE));
}
​

In the terminal run the following commands:

forge test --mt testCollectPresentNotChecked

forge test --mt testCheckListInexistentUser

Impact

  • inexistent users can mint SantasList ERC721 tokens come christmass 2023 block time on their first call to SantasList::collectPresent.

  • SantasList::getNaughtyOrNiceOnce and SantasList::getNaughtyOrNiceTwice will mislead the caller into believing that, inexistent users have been marked as NICE.

Tools Used

Manual review

Recommendations

in src/SantasList::Status add the following member as indicated in the below code comment.

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

Lead Judging Commences

inallhonesty Lead Judge over 1 year 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.