Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Lack of Access Control in checkList Function Leads to Potential Game Integrity Compromise

Summary

The SantasList contract's checkList function, intended to mark users as "Nice", "Extra Nice", or "Naughty", lacks proper access control. This issue allows any user, not just the designated "Santa", to mark others' status, potentially disrupting the intended game mechanics and fairness.

Vulnerability Details

In the SantasList contract, the checkList function is critical for determining whether users are eligible for Christmas presents. However, this function is marked as external without an onlySanta modifier, allowing any external account to call it. As a result, any user can set the status of any other user, bypassing the intended control that should be exclusively held by "Santa".

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

Impact

This vulnerability undermines the integrity of the game logic in the SantasList contract. Malicious users can mark themselves or others as "Nice" or "Extra Nice" without proper authorization, leading to unearned rewards. Conversely, they could also sabotage other users by marking them as "Naughty".

POC

  1. Exploit Test Case: A test case is created where a non-Santa user calls checkList and successfully changes another user's status.

  2. The non-Santa user (address user) calls checkList, marking their own status as "Nice".

  3. Santa calls checkTwice, confirming the status set by the non-Santa user.

  4. The test case asserts that the status change has been accepted and reflected in the contract, demonstrating the vulnerability.

function testCheckListByNotSanta() public {
vm.prank(user);
santasList.checkList(user, SantasList.Status.NICE);
assertEq(uint256(santasList.getNaughtyOrNiceOnce(user)), uint256(SantasList.Status.NICE));
vm.prank(santa);
santasList.checkTwice(user, SantasList.Status.NICE);
assertEq(uint256(santasList.getNaughtyOrNiceTwice(user)), uint256(SantasList.Status.NICE));
}

Tools Used

Foundry

Recommendations

Implement an onlySanta modifier to restrict the checkList function's access to the designated Santa account. This modifier should check if msg.sender is equal to the Santa's address and revert if not.

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