Missing Access Control on checkList Allows Anyone to Manipulate the Naughty/Nice List
Description
checkList is intended to be callable only by Santa (the deployer), as its counterpart checkTwice is protected with an
onlySanta modifier. Together they form the two-step verification before anyone can collect a present.
checkList is missing the onlySanta modifier entirely, meaning any address can call it to write any status for any
person into s_theListCheckedOnce. An attacker can self-promote to EXTRA_NICE, overwrite a legitimate user's status to
NAUGHTY to grief them, or front-run Santa's checkList call to force a mismatch that makes checkTwice permanently revert
for that address.
// @> No onlySanta modifier — anyone can call this
function checkList(address person, Status status) external {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
// For comparison — checkTwice is correctly protected
function checkTwice(address person, Status status) external onlySanta {
if (s_theListCheckedOnce[person] != status) {
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}
Risk
Likelihood:
Any deployed instance is exploitable from the first block — no setup or capital is required, only a single transaction
with gas cost
An attacker monitoring the mempool can front-run Santa's legitimate checkList(victim, NAUGHTY) call by submitting
checkList(victim, NICE) first, causing Santa's subsequent checkTwice(victim, NAUGHTY) to revert permanently
Impact:
Attackers can set themselves to EXTRA_NICE and, if Santa confirms, collect both the NFT and SantaTokens they did not
earn
Legitimate users can be permanently blocked from receiving presents by an attacker overwriting their status to NAUGHTY
before Santa's second check
Proof of Concept
function testCheckListMissingAccessControl() public {
address attacker = makeAddr("attacker");
// Attacker calls checkList with no Santa privileges
vm.prank(attacker);
santasList.checkList(attacker, SantasList.Status.EXTRA_NICE);
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(attacker)),
uint256(SantasList.Status.EXTRA_NICE)
);
// Attacker also griefs a legitimate user
address victim = makeAddr("victim");
vm.prank(attacker);
santasList.checkList(victim, SantasList.Status.NAUGHTY);
// Santa now cannot mark victim as NICE — checkTwice will revert
vm.prank(santa);
vm.expectRevert(SantasList.SantasList__SecondCheckDoesntMatchFirst.selector);
santasList.checkTwice(victim, SantasList.Status.NICE);
}
Recommended Mitigation
function checkList(address person, Status status) external {
function checkList(address person, Status status) external onlySanta {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
## Description With the current design of the protocol, anyone is able to call `checkList` function in SantasList contract, while documentation says only Santa should be able to call it. This can be considered as an access control vulnerability, because not only santa is allowed to make the first check. ## Vulnerability Details An attacker could simply call the external `checkList` function, passing as parameter the address of someone else and the enum Status `NAUGHTY`(or `NOT_CHECKED_TWICE`, which should actually be `UNKNOWN` given documentation). By doing that, Santa will not be able to execute `checkTwice` function correctly for `NICE` and `EXTRA_NICE` people. Indeed, if Santa first checked a user and assigned the status `NICE` or `EXTRA_NICE`, anyone is able to call `checkList` function again, and by doing so modify the status. This could result in Santa unable to execute the second check. Moreover, any malicious actor could check the mempool and front run Santa just before calling `checkTwice` function to check users. This would result in a major denial of service issue. ## Impact The impact of this vulnerability is HIGH as it results in a broken mechanism of the check list system. Any user could be declared `NAUGHTY` for the first check at any time, preventing present collecting by users although Santa considered the user as `NICE` or `EXTRA_NICE`. Santa could still call `checkList` function again to reassigned the status to `NICE` or `EXTRA_NICE` before calling `checkTwice` function, but any malicious actor could front run the call to `checkTwice` function. In this scenario, it would be impossible for Santa to actually double check a `NICE` or `EXTRA_NICE` user. ## Proof of Concept Just copy paste this test in SantasListTest contract : ``` function testDosAttack() external { vm.startPrank(makeAddr("attacker")); // any user can checList any address and assigned status to naughty // an attacker could front run Santa before the second check santasList.checkList(makeAddr("user"), SantasList.Status.NAUGHTY); vm.stopPrank(); vm.startPrank(santa); vm.expectRevert(); // Santa is unable to check twice the user santasList.checkTwice(makeAddr("user"), SantasList.Status.NICE); vm.stopPrank(); } ``` ## Recommendations I suggest to add the `onlySanta` modifier to `checkList` function. This will ensure the first check can only be done by Santa, and prevent DOS attack on the contract. With this modifier, specification will be respected : "In this contract Only Santa to take the following actions: - checkList: A function that changes an address to a new Status of NICE, EXTRA_NICE, NAUGHTY, or UNKNOWN on the original s_theListCheckedOnce list." The following code will resolve this access control issue, simply by adding `onlySanta` modifier: ``` function checkList(address person, Status status) external onlySanta { s_theListCheckedOnce[person] = status; emit CheckedOnce(person, status); } ``` No malicious actor is now able to front run Santa before `checkTwice` function call. The following tests shows that doing the first check for another user is impossible after adding `onlySanta` modifier: ``` function testDosResolved() external { vm.startPrank(makeAddr("attacker")); // checklist function call will revert if a user tries to execute the first check for another user vm.expectRevert(SantasList.SantasList__NotSanta.selector); santasList.checkList(makeAddr("user"), SantasList.Status.NAUGHTY); vm.stopPrank(); } ```
The contest is live. Earn rewards by submitting a finding.
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsThe contest is complete and the rewards are being distributed.