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

Attacker can collect unlimited presents

Martin7777

Summary

Attacker can call collectPresent function unlimited times after transfering owned SantaList tokens to other account.

Vulnerability Details

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
@> if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}

If attacker transfer owned tokens to other address this check will be passed.

Impact

Attacker can mint unlimited amount of ERC20 and ERC721 tokens.

Tools Used

  • foundry

Recommendations

Create mapping which tracks if user has already called collectPresent function.

+ mapping(address person => bool collected) private s_userCollected

Change if statement to check if user already called collectPresent function by checking the mapping value.

- if (balanceOf(msg.sender) > 0)
+ if (s_userCollected[msg.sender])

Don't forget to update mapping to true value after user successfully call collectPresent function for the first time.

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
if (
s_theListCheckedOnce[msg.sender] == Status.NICE &&
s_theListCheckedTwice[msg.sender] == Status.NICE
) {
+ s_userCollected[msg.sender] = true;
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE &&
s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
+ s_userCollected[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.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.