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