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

Missing 'onlySanta' modifier on 'checkList' function

luiscfaria

Summary

The SantasList.sol contract lacks appropriate access control for the checkList function, allowing any user, not just the designated "Santa", to modify the status of any address in the system. This oversight undermines the integrity of the list and the associated token distribution.

Vulnerability Details

The vulnerability lies in the checkList function of the SantasList smart contract. The function is designed to modify an address's status on the s_theListCheckedOnce list, which is a critical operation for determining who is eligible to receive NFTs and SantaTokens.

However, the function is missing a security modifier to restrict its execution to only the authorized "Santa" address. As a result, any external user can call this function and arbitrarily change the status of any address, leading to potential exploitation where ineligible users could be marked as 'NICE' or 'EXTRA_NICE'.

SantasList.sol:checklist:

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

Impact

The lack of access control in the checkList function of the SantasList smart contract presents significant risks, primarily due to two types of potential abuses:

  • Self-Benefiting Abuse: Users can exploit this vulnerability to change their own status to 'NICE' or 'EXTRA_NICE', making them one step closer to unjustly claim NFTs and SantaTokens.

  • Malicious Targeting of Others: Malicious actors can use this vulnerability to detrimentally alter the status of other users. For instance, they could change the status of legitimate 'NICE' or 'EXTRA_NICE' individuals to 'NAUGHTY' or 'UNKNOWN', unjustly depriving them of their rightful rewards.

In both scenarios, the integrity of the SantasList system is severely compromised. The platform's core principle of rewarding users based on their designated status is rendered ineffective, leading to widespread mistrust and potentially significant economic repercussions for both the users and the platform.

Proof of Concept

Working test case

The following test uses the attacker address to set the users status to NAUGHTY after Santa already updated it to EXTRA_NIC. When run, this test will pass, demonstrating that the attacker can modify the mapping:

function test_Fail_CantCollectPresentExtraNiceAfterAttack() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.startPrank(attacker);
santasList.checkList(user, SantasList.Status.NAUGHTY);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
vm.expectRevert();
santasList.collectPresent();
}

Terminal:

[PASS] test_Fail_CantCollectPresentExtraNiceAfterAttack() (gas: 73239)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.91ms

Tools Used

Manual Code Review

Recommended Mitigation

Add the onlySanta modifier to the function.

SantasList.sol:checklist:

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.