First Flight #5: Santa's List

Summary

Status is NICE by default. This allows user to collectPresent() without checkList and checkTwice calls.

Vulnerability Details

Because NICE is stated first inside Status enum, the default value for Status is NICE.
This makes Status initially NICE for every person inside mappings s_theListCheckedOnce and s_theListCheckedTwice.
Keeping this in mind, every user can retrieve present NFT by calling collectPresent() if status was not set with checkList and checkTwice calls.

https://github.com/Cyfrin/2023-11-Santas-List/blob/main/src/SantasList.sol#L69-L74

Modified test case provided to demonstrate the issue:

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

Allows every User to collect NFT if status was not set by Santa with checkList and checkTwice calls.

Tools Used

Foundry

Recommended Mitigation

Correct enum Status constants order.
Make NOT_CHECKED_TWICE status default.

enum Status {
NOT_CHECKED_TWICE,
NICE,
EXTRA_NICE,
NAUGHTY
}