First Flight #5: Santa's List

Summary

There are several places where the documentation of the protocol and the comments in the codebase conflict.

Vulnerability Details

The documentation says that the SantasList::checkList function changes an address to a new Status of NICE, EXTRA_NICE, NAUGHTY, or UNKNOWN on the original s_theListCheckedOnce list and the SantasList::checkTwice function 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. But in the comments in the codebase is written that these two functions do first and second pass on someone if they are naughty or nice. And this is incorrect because the code implementation allos Santa to change the status of the given address and emit an event with the address and status. I believe that the documentation says the intended behaviour.

The documentation says:
"collectNFT: A function that allows a NICE or EXTRA_NICE user to collect their NFT. EXTRA_NICE users also receive SantaToken which is used to purchase an additional NFTs. An address is only allowed to collect 1 NFT per address, there is a check in the codebase to prevent someone from minting duplicate NFTs."
In the codebase there is not a function with name collectNFT, probably this is the function SantasList::collectPresent.

For the next function SantasList::buyPresent the documentation says:
"buyPresent: A function that trades 2e18 of SantaToken for an NFT. This function can be called by anyone."
But the comments in the codebase says:
"@notice Buy a present for someone else. This should only be callable by anyone with SantaTokens."
But the people with SantaTokens are these people which have status EXTRA_NICE. The documentation says that the function trades 2e18 of SantaToken for an NFT but the comments says that the function is used to buy a present for someone else. Actually, the function can be used to trade SantaToken for an NFT, if the caller write his address as presentReceiver. But the function will burn 1e18 of SantaToken for an NFT. Also, if the naughty people call this function, they have not SantaTokens and for their NFT should pay another person with SantaToken. Additionally, in the codebase is written that the naughty people pay more for present which is not implemented and not documented.

Impact

These differences between the documentation, comments in the codebase and implementation confuse a lot. It is hard to determine what actually is the intended behaviour of the protocol and how to implement the functions in correct way.

Tools Used

VS Code

Recommendations

Rewrite the documentation and comments in the codebase in a way that they are no contradicted and regarding to the intended behaviour of the protocol.