First Flight #5: Santa's List
First Flight #5
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

`SantasList::checkList` missing modifier allows anyone to add any `status` to any `person`

Oozman

SantasList::checkList missing modifier allows anyone to add any status to any person

Summary

SantasList::checklist is callable by any user, putting the first check of users' list at risk of being manipulated.

Vulnerability Details

SantasList::checklist is supposed to be callable only by santa.
The function does not have the appropriate modifier in order to make sure it is only callable by him.
Any user can call this function.
Any user could be given any type of behaviour by anyone.
The business logic of the contract states that only Santa Claus has the right and power to judge people.

Impact

LOW Impact:

Because there is a second check of the list of persons expressed by the function SantasList::checkTwice.
This second function has the appropriate onlySanta modifier ensuring that only santa can call it.
Furthermore, there is a revert if the person's status differs from the one passed in SantasList::checkList
Since both checks need to give similar behavior to persons, even if a malicious person who didn't behave gave himself a first false behaviour, only santa can give the second check and thus validate the meritance of a present.

HIGH Probability:

Because SantasList::checkList is external and can be called by anyone. No need to be too curious to try it and give oneself anytype of behaviour.

Tools Used

Manual review.

The following test script, giving a [PASS]:

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

Recommendations

- function checkList(address person, Status status) external {
+ 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.