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

Unchecked persons are eligible for a present

mircha

Summary

The default status of the persons is ǸICEwhich, unless Santa is checking them, will make them eligible for a present.

Vulnerability Details

In Solidity, the default mapping is the default value of that type. The default value of an Enum is 0 so in the case of the SantasList contract, the two mappings s_theListCheckedOnce and s_theListCheckedTwice will have Status.NICE as the default mapping.

If Santa doesn't manage to check everybody till Christmas, unchecked people could still get a present.

Also the existing test still passes if we comment out the lines used to check the user:

function testCollectPresentNice() public {
// vm.startPrank(santa);
// santasList.checkList(user, SantasList.Status.NICE);
// santasList.checkTwice(user, SantasList.Status.NICE);
// vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
vm.stopPrank();
}

Impact

High as unchecked people and, in the worst case, naughty people could get a present.

Tools Used

Manual code review

Recommendations

Change the order of the Enum values. NOT_CHECKED_TWICE could be renamed to NOT_CHECKED as it's more meaningful.

enum Status {
NOT_CHECKED,
NICE,
EXTRA_NICE,
NAUGHTY
}
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!