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

DoS via `checkList` front running

alexbabits

Summary

Anyone can call checkList and set the status of a person because it is missing the onlySanta modifier. This allows a malicious person to front run a collectPresent call and DoS a NICE or EXTRA_NICE user attempting to collect their present(s) by changing their status to NAUGHTY.

PoC

  1. Santa calls checkList and sets cindyLouWho to EXTRA_NICE in s_theListCheckedOnce mapping.

  2. Santa calls checkTwice and sets cindyLouWho to EXTRA_NICE in s_theListCheckedTwo mapping.

  3. cindyLouWho attempts to call collectPresent to collect her free NFT and token.

  4. theGrinch is looking for collectPresent calls in the mempool. When he sees cindyLouWho attempt to collect her present, his MEV bot can quickly call checkList, passing in cindryLouWho and NAUGHTY, causing the s_theListCheckedOnce mapping to update her status to NAUGHTY. Because the mappings do not match, the status checks will fail inside of collectPresent for cindyLouWho, and she will not be able to get her free gifts.

  5. Santa will have to manually set her status back to EXTRA_NICE, but theGrinch can frontrun her again, unless Santa has a faster MEV bot than theGrinch.

Add this test to the existing suite.

address cindyLouWho = makeAddr("cindyLouWho");
address theGrinch = makeAddr("theGrinch");
​
function test_DoS_User() public {
vm.startPrank(santa);
santasList.checkList(cindyLouWho, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(cindyLouWho, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(theGrinch);
// This call executes before cindyLouWho's collectPresent call due to frontrunning
santasList.checkList(cindyLouWho, SantasList.Status.NAUGHTY);
vm.stopPrank();
​
vm.startPrank(cindyLouWho);
vm.expectRevert(SantasList.SantasList__NotNice.selector);
santasList.collectPresent();
}

Impact

Users can get DoS'ed by a malicious actor who frontruns their collectPresent call by calling checkList and changing their status to NAUGHTY.

Tools Used

Manual Review

Recommendations

Add the onlySanta modifier to the checkList function.

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.