Summary
checkList() and checkTwice() can be called many times without any limit to it.
Vulnerability Details
the status of a person can be changed multiple times in checkList(), once it has been set it can still be changed with no limit to how many times this function can be called.
Impact
checkList() could cause unnecessary changes to the s_theListCheckedOnce when its not needed.
function testCheckListHasNoLimitToHowManyTimesItCanBeCalled() public {
vm.prank(santa);
santasList.checkList(user, SantasList.Status.NICE);
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(user)),
uint256(SantasList.Status.NICE)
);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(user)),
uint256(SantasList.Status.EXTRA_NICE)
);
santasList.checkList(user, SantasList.Status.NAUGHTY);
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(user)),
uint256(SantasList.Status.NAUGHTY)
);
santasList.checkList(user, SantasList.Status.NOT_CHECKED_TWICE);
assertEq(
uint256(santasList.getNaughtyOrNiceOnce(user)),
uint256(SantasList.Status.NOT_CHECKED_TWICE)
);
}
doesn't really affect checkTwice() as much as it affects checkList() because checkTwice() won't work anyways unless it matches with the value from s_theListCheckedOnce - it would just be good practice to have a modifer as well in this function..
Tools Used
Manual Review
Foundry Test
Recommendations
add a modifier to both the functions, once it has been called once don't allow them to be called again.
+ error SantasList__FirstCheckAlreadyDone();
+ error SantasList__SecondCheckAlreadyDone();
function checkList(address person, Status status) external {
+ if(uint256(s_theListCheckedOnce[person]) != 0) {
+ revert SantasList__FirstCheckAlreadyDone();
+ }
s_theListCheckedOnce[person] = status;
emit CheckedOnce(person, status);
}
function checkTwice(address person, Status status) external onlySanta {
+ if(uint256(s_theListCheckedTwice[person]) != 0) {
+ revert SantasList__SecondCheckAlreadyDone();
+ }
if (s_theListCheckedOnce[person] != status) {
revert SantasList__SecondCheckDoesntMatchFirst();
}
s_theListCheckedTwice[person] = status;
emit CheckedTwice(person, status);
}
