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

SantasList:checkList() missing onlySanta modifier allows non-Santa user to conduct first check, potentially resulting in person receiving NFT with only one Santa conducted check

Summary

checkList() missing onlySanta modifier allows non-Santa user to conduct first check, potentially resulting in person receiving NFT with only one Santa conducted check

Vulnerability Details

SantasList:checkList() can be executed by a non-Santa user, contrary to the protocol documentation. This permits a person's first check to be done by a non-Santa user and potentially allowing Santa to conduct a 2nd check on a person whose first check was not done as expected. The result would be that the person would receive the NFT when they actually should not.

Impact

Low overall impact (Impact: Medium, Likelihood: Low)
Would require Santa to do a 2nd check on a person for whom he did not conduct a 1st check.

Tools Used

Visual Studio Code, Foundry

PoC

The following Foundry test will fail with current code due to the non-Santa user's first check not getting reverted...

function testCheckListNonSanta() public {
vm.prank(user);
vm.expectRevert();
santasList.checkList(user, SantasList.Status.NICE);
vm.stopPrank();
}

Recommendations

Add the onlySanta modifier to the SantasList:checkList() function, as shown below...

function checkList(address person, Status status) external onlySanta {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
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!