First Flight #5: Santa's List

Summary

Any user declared as NICE or EXTRA_NICE can get undefined amount of presents by calling 'collectPresent()' function in SantasList.sol.

Vulnerability Details

The 'collectPresent()' function checks whether the balance of an address is greater than 0 (line 151 in SantasList.sol) in order to allow them claim for the present or not. After claiming for the first time, a user can use 'transferFrom()' function from ERC721.sol to send their present to another address, this restarts its balance to 0 which allows them to collect from a new present whenever they want to.

Impact

The impact is really risky, as users are allowed to mint undefined number of presents (such an impossible work for Santa), instead of one as they are supposed to.

Tools Used

Solidity

Recommendations

A better way of checking if an address has claimed for their present would be a mapping as a state variable:

mapping (address => bool) presentCollected;

Then in line 151 of SantasList.sol instead of checking balance it is necessary to check this mapping for the claimer:

if (presentCollected[msg.sender]){
revert SantasList__AlreadyCollected();
}

If the present has not been claimed, after the call of '_mintAndIncrement()' function we will set the mapping as 'true', both for NICE or EXTRA_NICE users, to prevent them from calling the function multiple times:

_mintAndIncrement();
presentCollected[msg.sender] = true;