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

Inadequate Verification of Distributed Presents in `collectPresent()`

n0kto

Summary

The collectPresent function in the smart contract does not adequately verify if a person has already collected a present. This allows users to transfer their NFT to another address and collect as many present as he wants.

Vulnerability Details

The vulnerability arises from the lack of proper verification in the collectPresent function. The function only check if currently the user has a present, which is not the same that "user have already collected their gift". This enables users to transfer their NFT to any other address and collect presents infinitely.

PoC

function testDontVerifyEnoughPresentDistributedInCollectPresent() 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);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
​
//Transfer previous present and collect a new one
santasList.transferFrom(user, user_2, 0);
assertEq(santasList.ownerOf(0), user_2);
assertEq(santasList.balanceOf(user), 0);
​
santasList.collectPresent();
​
//verify both user have a present thanks to one nice person
assertEq(santasList.balanceOf(user), 1);
assertEq(santasList.balanceOf(user), 1);
​
vm.stopPrank();
}

Impact

By exploiting this vulnerability, "Nice" or "Extra-Nice" users can collect multiple presents by transferring their NFT to different addresses. This can lead to an unfair distribution of presents or a steal of all presents minting all possible NFTs.

Additionnaly, with this bad verification, If one user send a present to anyone who didn’t collect their present, this person won’t be able to collect the present they deserve.

Tools Used

Manual review

Recommendations

To mitigate this vulnerability, it is recommended to implement proper verification in the collectPresent function. The function should check if a person has already collected a present and prevent them from collecting another one. This can be achieved by creating a mapping to keep track of addresses that have already collected a present.

Here is a possible solution :

mapping(address => bool) private s_alreadyCollected;
​
function collectPresent2() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (s_alreadyCollected[msg.sender]) {
revert SantasList__AlreadyCollected();
}
if (
s_theListCheckedOnce[msg.sender] == Status.NICE &&
s_theListCheckedTwice[msg.sender] == Status.NICE
) {
s_alreadyCollected[msg.sender] = true;
_mintAndIncrement();
return;
} else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE &&
s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {
s_alreadyCollected[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.