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

Multiple gift claim

f3d0ss

Vulnerability Details

Calling collectPresent addresses should not be able to collect more than once. This is apparently enforced by enforced by

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

This will not work if a misjudge NICE address send his NFT to another address before calling collectPresent a second time.
The next test should fail if the protocol work correctly:

function testMultipleGiftClaim() public {
uint256 mintedGiftId = 0;
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.startPrank(user);
santasList.collectPresent();
santasList.transferFrom(user, secondAddress, mintedGiftId);
santasList.collectPresent();
}

Impact

A NICE attacker can claim an "infinite" amount of gift.

Recommendations

Use a mapping mapping(address person => bool claimed) private s_claimed; to keep track of addresses that already claimed the gift.
Replace the check:

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

With:

if (s_claimed[msg.sender]) {
revert SantasList__AlreadyCollected();
}

And in the collectPresent function in the lines before the _mintAndIncrement(); add s_claimed[msg.sender] = true;

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!