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

checkList() can be called by anyone.

Summary

checkList() can be called by anyone when this function should have restricted acces.

Vulnerability Details

In Santalist.sol, the checkList() is designed to set the initial status of a user. However onlySanta() modifier is absent, and how is mentioned in the comments the function only can be called by santa. This omission allows access to the function for anyone.

function checkList(address person, Status status) external {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}

POC

function testAnyoneCanCallcheckList() public {
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
santasList.checkList(attacker, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
}

Impact

This error introduces two potential vulnerabilities:

  1. An attacker may exploit the event CheckedOnce(address person, Status status) by spamming it.

  2. An attacker could frontrun a user invoking collectPresent(), attempting to claim their present by modifying, for instance, s_theListCheckedOnce to the NAUGHTY status. This manipulation may result in the user's transaction failing, as it checks for:

if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE)
else if (s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE && s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE)

Tools Used

Manual review.

Recommendations

Add onlySanta() modifier to checkList().

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.