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

checkList` Lacks Access Control and `checkTwice` Cannot Override a Prior Status, Permanently Locking User Eligibility

web3mio

Description

Two independent but interacting design flaws combine to make user eligibility permanently irrecoverable:

Flaw 1 — checkList has no access control.

The NatSpec comment states "Only callable by santa", but the implementation has no onlySanta modifier:

// SantasList.sol:122-125
function checkList(address person, Status status) external { // no modifier
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}

Any address in the world can call checkList and overwrite any person's first-check status at any time — including after checkTwice has already finalised their eligibility.

Flaw 2 — checkTwice enforces a strict match against the current first-check value and cannot override it.

// SantasList.sol:134-140
function checkTwice(address person, Status status) external onlySanta {
if (s_theListCheckedOnce[person] != status) {
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}

Santa cannot call checkTwice with a status that differs from s_theListCheckedOnce. This means:

  • A user checked NAUGHTY on the first pass cannot be upgraded to NICE or EXTRA_NICE via checkTwice alone.

  • If anyone poisons s_theListCheckedOnce (via Flaw 1) after checkTwice has already run, the two mappings are now mismatched and checkTwice cannot repair them — it would have to match the poisoned value.

The consequence in collectPresent:

collectPresent requires both mappings to agree on the same eligible status:

// SantasList.sol:157-166
if (s_theListCheckedOnce[msg.sender] == Status.NICE
&& s_theListCheckedTwice[msg.sender] == Status.NICE) { ... }
else if (s_theListCheckedOnce[msg.sender] == Status.EXTRA_NICE
&& s_theListCheckedTwice[msg.sender] == Status.EXTRA_NICE) { ... }
revert SantasList__NotNice();

Any mismatch — regardless of whether either individual status is NICE or EXTRA_NICE — causes a revert. There is no code path that allows Santa to repair a mismatched state without calling checkList first (which anyone else can immediately re-poison).

Attack Scenarios

Scenario A — Santa cannot upgrade a NAUGHTY classification.

  1. Santa calls checkList(alice, NAUGHTY) and checkTwice(alice, NAUGHTY).

  2. Alice's behaviour improves. Santa wants to upgrade her to NICE.

  3. Santa calls checkTwice(alice, NICE) → reverts (SecondCheckDoesntMatchFirst).

  4. To fix this, Santa must call checkList(alice, NICE) first — but since checkList has no access control, an attacker can immediately overwrite it again.

Scenario B — Attacker permanently blocks an approved user.

  1. Santa calls checkList(alice, NICE) then checkTwice(alice, NICE). Alice is eligible.

  2. Attacker calls checkList(alice, NAUGHTY) — overwrites s_theListCheckedOnce to NAUGHTY.

  3. Now s_theListCheckedOnce[alice] = NAUGHTY, s_theListCheckedTwice[alice] = NICE. They disagree.

  4. Alice tries collectPresent() → reverts (NotNice).

  5. Santa tries checkTwice(alice, NICE) to repair → reverts (SecondCheckDoesntMatchFirst, because the first check is now NAUGHTY).

  6. Santa would have to call checkList(alice, NICE) again, but the attacker can immediately re-call checkList(alice, NAUGHTY) in a front-run. The attacker can loop this indefinitely at negligible gas cost.

Impact

  • Any legitimately approved user can be permanently barred from collecting their present by any third party calling the free checkList function.

  • Santa has no on-chain mechanism to override a poisoned first-check status in a front-run resistant way.

  • A user checked NAUGHTY in error can never be upgraded through checkTwice alone — the fix requires a new checkList call that itself has no protection.

Proof of Concept

Added to test/unit/SantasListTest.t.sol:

Sub-impact A — Santa cannot upgrade a NAUGHTY user:

function testSantaCannotUpgradeNaughtyUser() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NAUGHTY);
santasList.checkTwice(user, SantasList.Status.NAUGHTY);
​
// Santa wants to upgrade user to NICE — checkTwice blocks it
vm.expectRevert(SantasList.SantasList__SecondCheckDoesntMatchFirst.selector);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
​
// User remains permanently locked as NAUGHTY
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.prank(user);
vm.expectRevert(SantasList.SantasList__NotNice.selector);
santasList.collectPresent();
}

Sub-impact B — Attacker poisons an approved user's first-check status:

function testCheckListPoisonsApprovedUser() public {
// Santa legitimately approves user
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.NICE);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
​
// Attacker calls the unguarded checkList to corrupt first-check status
address attacker = makeAddr("attacker");
vm.prank(attacker);
santasList.checkList(user, SantasList.Status.NAUGHTY);
​
// s_theListCheckedOnce = NAUGHTY, s_theListCheckedTwice = NICE — mismatched
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
vm.prank(user);
vm.expectRevert(SantasList.SantasList__NotNice.selector);
santasList.collectPresent();
​
// Santa cannot repair via checkTwice — it must match the poisoned NAUGHTY value
vm.startPrank(santa);
vm.expectRevert(SantasList.SantasList__SecondCheckDoesntMatchFirst.selector);
santasList.checkTwice(user, SantasList.Status.NICE);
vm.stopPrank();
}

Result: Both tests pass, confirming that (A) Santa cannot correct a status via checkTwice alone, and (B) any third party can permanently block an approved user.

Recommended Mitigation

Fix 1 — Add onlySanta to checkList.

The NatSpec already documents this intent; the implementation just needs to match it:

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

This closes the poisoning vector (Scenario B) entirely.

Fix 2 — Allow Santa to correct a second-check status independently of the first.

Remove the strict equality requirement so Santa can update either check independently:

function checkTwice(address person, Status status) external onlySanta {
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}

If the business requirement is that both checks must agree before a user is eligible, enforce that agreement in collectPresent (or during report generation off-chain) rather than preventing Santa from ever setting a corrective value.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-01] Anyone is able to call `checkList` function in SantasList contract and prevent any address from becoming `NICE` or `EXTRA_NICE` and collect present.

## 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(); } ```

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!