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 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.