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

checkList function not being set to onlySanta modifier, the checkList function can be call by any user.

0xrolko

Summary

The function SantasList::checkList(address person, Status status) can be call only by santa,
therefore the modifier SantasList::onlySanta() must be applied to it.

Vulnerability Details

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

If a function must only be called by the person who owns the contract,
in this case santa, not applying this restriction to the checkLIst function allows
anyone to call this function.
By using the onlySanta modifier, we ensure that the checkLIst function
can and must only be called by the owner of the contract.

Impact

Each user can change their status themselves and assign themselves the highest status,
which will enable them to acquire SantaTokens and NFT.
 On the other hand, anyone can access the `checkList` function and change the status of
 any user by assigning it any value they wish.
// Here is my proof of code
/** For example: When a unit test is run on the checkList() function, changing the user who owns the contract (santa)
to a simple user (user) and assigning the highest status that allows the SantaTokens to be won,
the test is run without any errors. */
function testCheckList() public {
- vm.prank(santa);
- santasList.checkList(user, SantasList.Status.NICE);
assertEq(uint256(santasList.getNaughtyOrNiceOnce(user)), uint256(SantasList.Status.EXTRA_NICE));
}
function testCheckList() public {
+ vm.prank(user);
+ santasList.checkList(user, SantasList.Status.EXTRA_NICE);
assertEq(uint256(santasList.getNaughtyOrNiceOnce(user)), uint256(SantasList.Status.EXTRA_NICE));
}
​
## Tools Used
-Foundry
## Recommendations
​
apply the `onlySanta` modifier to the `checkList` function.
​
```diff
- function checkList(address person, Status status) external {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
​
+ function checkList(address person, Status status) external onlySanta {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
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.