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 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.