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

`SantaList::checkList(address,Satatus)` missing `onlySanta` modifier

wasny

Summary

The checkList(address,Satatus) function can be called by everyone. Therefore, it should be marked as onlySanta modifier.

Vulnerability Details

In order to maintain control authorization in the contract, it is essential to include modifiers to functions. Even though the checkList(address, Status) function is intended to be invoked only by the Santa user, the call operation is not verified with the onlySanta modifier. It is all open to any external calls. By implementing the onlySanta modifier in the checkList(address, Status) function, unauthorized calls can be prevented.

Impact

Anyone can change their first status.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import {SantasList} from "../../src/SantasList.sol";
import {SantaToken} from "../../src/SantaToken.sol";
import {Test} from "forge-std/Test.sol";
import {Utilities} from "./utils/Utilities.sol";
​
contract SantasListTest is Test {
Utilities internal utils;
SantasList internal santasList;
SantaToken internal santaToken;
address payable internal user2;
address payable internal user;
address payable internal santa;
​
function setUp() public {
utils = new Utilities();
address payable[] memory users = utils.createUsers(3);
user2 = users[0];
user = users[1];
santa = users[2];
vm.label(santa, "Santa");
vm.label(user, "User");
vm.label(user2, "Attacker");
vm.startPrank(santa);
santasList = new SantasList();
santaToken = SantaToken(santasList.getSantaToken());
vm.stopPrank();
}
​
function testExploit() public {
vm.prank(user);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
assertEq(uint256(santasList.getNaughtyOrNiceOnce(user)), uint256(SantasList.Status.EXTRA_NICE));
}
​
}

Finally run the test with: forge test

Tools Used

  • Foundry

Recommendations

checkList(address,Satatus) should be marked as onlySanta.

- function checkList(address person, Status status) external {
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
​
+ 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.