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

Potential Block and Gas Consumption Issue in `collectPresent()`

n0kto

Summary

The collectPresent function contains unnecessary double checks that can be optimized to avoid potential bugs and reduce gas consumption.

Vulnerability Details

The collectPresent function has two conditional checks that verify the status of the caller in both s_theListCheckedOnce and s_theListCheckedTwice mappings. However, these double checks are redundant and can be simplified to improve efficiency.

The first issue is that the second check (s_theListCheckedTwice[msg.sender]) is unnecessary because it can only be true if the first check (s_theListCheckedOnce[msg.sender]) is already true. Therefore, the second check can be removed to simplify the logic and avoid potential bugs.

The second issue is that the first mapping can be changed even after the second mapping is set, leading to blocking a person to collect their present.

Foundry PoC

function testBlockPresentCollection() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
santasList.checkList(user, SantasList.Status.NAUGHTY);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.prank(user);
vm.expectRevert();
santasList.collectPresent();
}

Impact

  • Could block any person if Santa change the first mapping after the second is set (or an other vulnerability is used to change the first mapping)

  • Useless gas consumption

Tools Used

Manual review

Recommendations

It is recommended to simplify the conditional checks and use only s_theListCheckedTwice. Here is an updated version of the function:

function collectPresent() external {
if (block.timestamp < CHRISTMAS_2023_BLOCK_TIME) {
revert SantasList__NotChristmasYet();
}
if (balanceOf(msg.sender) > 0) {
revert SantasList__AlreadyCollected();
}
if (s_theListCheckedTwice[msg.sender] == Status.NICE) {
_mintAndIncrement();
return;
} else if (s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE) {
_mintAndIncrement();
i_santaToken.mint(msg.sender);
return;
}
revert SantasList__NotNice();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.