Summary

The SantasList contract assumes that "only " santa can modify his list. The checkList() function modifies the s_theListCheckedOnce storage variable, where santa puts an address on his list and changes status, but the function doesn't include access control meaning that anyone, including a malicious actor, can put any address or their own, or change status.

Vulnerability Details

This vulnerability exists in the checkList() function in the SantasList.sol file starting line 121

The checkList function includes no access controls meaning that anyone can call it and modify the list:

To restrict who can modify the list, there needs to be a check that the function caller, the msg.sender, is the owner of the contract, i.e. i_santa

Impact

The checkList function in the SantasList.sol contract allows anyone to update the list, which could lead to unauthorized changes or malicious activities.

For example, a malicious actor could add their own name to the list, or change the status of someone else's name to naughty. This could have a significant impact on the integrity of the list and could lead to people being unfairly rewarded or punished.

As per the following NatSpec comment: Only callable by santa i.e. only i_santa the owner can modify the list is the core assumption, and functionality that does not hold, this is a high severity vulnerability.

Proof of Concept

Working Test Case

The makeAddr helper function is used to setup an attacker address to call the checkList() function:

The following test, will set an address "attacker" and change the status to NICE. When run, this test will pass, demonstrating that the attacker can modify the list:

Run the test:

Which yields the following output:

Recommendation Mitigation

Include access control to restrict who can call the checkList function to be only the owner: i_santa. This can be achieved by 2 ways:

• using the onlySanta modifier

• Using an access modifier e.g. OpenZeppelin's onlyOwner. To use this modifier, the SantasList contract will need to inherit from OpenZeppelin's Ownable contract and call it's constructor inside the constructor of SantasList:

As per the OpenZeppelin documentation, by default, the owner of an Ownable contract is the account that deployed it, meaning that the s_owner state variable can be removed.

Using onlyOwner modifier(similar to the onlySanta modifier) adds logic to check that the msg.sender is the owner of the contract before executing the function's logic:

Tools Used

• Forge