Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Any User Can Collect Presents

Summary

After the CHRISTMAS_2023_BLOCK_TIME arrives, any user can call SantaList::collectPresent to get the NFT without the need of Santa's eligibility.

Vulnerability Details

Enums in Solidity are initialized with the first member of the list. Currently the enum Status is declared as such:

enum Status {
NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

Due to this ordering, all accounts in the mappings s_theListCheckedOnce and s_theListCheckedTwice have a NICE status by default.

Proof of Concept

Paste the following test in SantaListTest.t.sol to test the vulnerability.

POC
function testSecurityReview__AnyAccountCanCollectPresent() public {
// assert user is checked twice by default
assertEq(uint256(santasList.getNaughtyOrNiceOnce(user)), 0);
assertEq(uint256(santasList.getNaughtyOrNiceTwice(user)), 0);
// jump in time
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
// user call functions
vm.prank(user, user);
santasList.collectPresent();
// assert user claimed NFT without prior authorization
assertEq(santasList.balanceOf(user), 1);
assertEq(santasList.ownerOf(0), user);
}

Impact

Users don't need their eligibility to be set by Santa to collect a present.

Tools Used

Foundry and VS Code.

Recommendations

Reorder the members of the enum as such:

enum Status {
NOT_CHECKED_TWICE
NICE,
EXTRA_NICE,
NAUGHTY,
}

With this order users will need to get their eligibility from Santa to claim presents.

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.