Summary

According to the documentation, Status enum has 4 possible values:

• NICE

• EXTRA_NICE

• NAUGHTY

• UNKNOWN

Documentation explains : "In this contract Only Santa to take the following actions:

• checkList: A function that changes an address to a new Status of NICE, EXTRA_NICE, NAUGHTY, or UNKNOWN on the original s_theListCheckedOnce list.

• checkTwice: A function that changes an address to a new Status of NICE, EXTRA_NICE, NAUGHTY, or UNKNOWN on the new s_theListCheckedTwice list only if someone has already been marked on the s_theListCheckedOnce. "

Nevertheless, current implementation of the enum is as follows:

NOT_CHECKED_TWICE should be replaced by UNKNOWN. Moreover, neither NICE or EXTRA_NICE should be in the first place of the enum, as it means every address defaults to this status which would break the contract (see other finding)).

Vulnerability Details

The issue consists of a lack of clarity about this enum. Documentation defines an UNKNOWN status, which doesn't exist in the contract, replaced by NOT_CHECKED_TWICE. Moreover, the first possible value of the enum should be UNKNOWN, making every address default to UNKNOWN before any Santa check.

Impact

The impact is LOW as there is no direct security implication about this finding. There are security implications about the order of enum elements but this is out of the scope of this finding.

Tools Used

Manual

Recommendations

I suggest to keep the 4 elements in the enum, with UKNOWN as the first one. This way, everyone has an UNKNOWN status by default, and can then be assigned to NICE, EXTRA_NICE or NAUGHTY status by Santa.