Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Missing onlySanta modifier on checkList allows anyone to set the initial status, potentially tricking Santa

Summary

While there is an onlySanta modifier, and it's applied to checkTwice, it's not applied to checkList.

Vulnerability Details

This allows anyone to add themselves, or update their status if they're already there.

Impact

Depending on Santa's workflow, the impact may be minimal, or it may be major. Since a user can pre-emptively add themselves to the list as NICE or EXTRA_NICE, if Santa were to look for those that weren't checked twice and perhaps auto-checking twice late on the 24th, it could allow tricking Santa. However, if Santa always just set someone's status and soon after, checked twice, the impact is minimal as Santa would overwrite the status previously set.

Tools Used

forge test. Wrote a test proving anyone can call checkList.

function testAnyoneCanCheckList() public {
vm.prank(user);
santasList.checkList(user, SantasList.Status.NICE);
assertEq(uint256(santasList.getNaughtyOrNiceOnce(user)), uint256(SantasList.Status.NICE));
}

Recommendations

Apply onlySanta modifier to checkList. Write missing negative tests (expecting reverts with custom error) for both checkList and checkTwice.

Updates

Lead Judging Commences

inallhonesty Lead Judge almost 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.