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

Anyone can execute `SantasList::checkList` function to change anyone's status

bhvrvt

Summary

As per the protocol's documentation, SantasList::checkList function is only intended to be executed by Santa to do a first pass on someone if they are naughty or nice. But there are no checks put in place in the code to ensure this condition.
Hence, a malicious user can execute this function to tamper with the status of any and all users on Santa's List as well as add new users with their desired status, compromising the integrity of data stored on the protocol.

Vulnerability Details

Following is the vulnerable piece of code in the SantasList::checkList function :

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

As is evident in the code, there are no checks in place to prevent users other than Santa to execute this function.

Proof of Concept

Actors

  • Santa: Deployer of the protocol.

  • Attacker: Any malicious user on the network other than Santa.

  • Victim: Any non-malicious user participating in Santa's List protocol.

Working Test Case

Write and run the following test case in the SantasListTest.t.sol test file.

function testAnyoneCanExecuteChecklistFunction() public {
// Defining Attacker's address
address attacker = makeAddr("attacker");
​
// Defining Victim's address
address victim = makeAddr("victim");
​
// Santa lists victim as `NICE`
vm.prank(santa);
santasList.checkList(victim, SantasList.Status.NICE);
​
// Santa lists attacker as `NAUGHTY`
vm.prank(santa);
santasList.checkList(attacker, SantasList.Status.NAUGHTY);
​
/////////// Attacker's POC ///////////
​
// Attacker changes victim to `NAUGHTY`
vm.prank(attacker);
santasList.checkList(victim, SantasList.Status.NAUGHTY);
​
// Attacker changes attacker to `NICE`
vm.prank(attacker);
santasList.checkList(attacker, SantasList.Status.NICE);
​
// This test returns true proving attacker was able to change both victim's and attacker's status
​
// Victim is now NAUGHTY instead of NICE
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(victim)),
uint256(SantasList.Status.NAUGHTY)
);
​
// Attacker is now NICE instead of NAUGHTY
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(attacker)),
uint256(SantasList.Status.NICE)
);
}

Impact

This vulnerability grants unauthorized access to any malicious actor on the network to perform following actions :

  • add more of their own addresses with NICE and EXTRA_NICE 'first pass' statuses to increase their chances of winning more prizes after the second pass.

  • change statuses of users already present in the protocol as per attacker's will and benefit.

Tools Used

Foundry

Recommendations

The vulnerable function needs the SantasList::onlySanta modifier added to it which is already defined in the protocol.

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