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

Arbitrary Burning of Tokens of any user in `buyPresent()`

n0kto

Summary

The buyPresent function in the SantasList.sol allows any user to burn tokens of an arbitrary address and mint a new NFT for themselves (msg.sender). This can lead to unauthorized stealing of presents that belong to other users. The function should only allow the owner of the tokens to burn them and mint NFTs.

Vulnerability Details

The vulnerability arises from the lack of proper ownership check in the buyPresent function. Any user can call this function and burn tokens belonging to an arbitrary address, as well as mint a new NFT for themselves. This means that an attacker can steal presents intended for other users by burning their tokens and claiming the presents for themselves.

Foundry PoC

function testAnyoneCanBuyPresentForThemself() public {
vm.prank(address(santasList));
santaToken.mint(user_2);
​
// user buy a present with the token of user_2
vm.prank(user);
santasList.buyPresent(user_2);
​
assertEq(santasList.balanceOf(user), 1);
}

Impact

By exploiting this vulnerability, an attacker can burn 1e18 SantaToken of any user and minting a new NFT for themself. It can be repeated on every user until they have less than 1e18 SantaToken. With bots, a good hacker can steal all the christmas presents before anyone collect their presents.
This can result in significant loss and frustration for the affected users, as well as undermine the fairness and integrity of the gift-giving process.

Tools Used

Manual review

Recommendations

The function should only allow the sender of the message to burn their token and mint a NFT for the arbitrary address.
Here is a possible solution :

function buyPresent(address presentReceiver) external {
i_santaToken.burnTokens(msg.sender);
_mintAndIncrementForSomeoneElse(presentReceiver);
}
​
function _mintAndIncrementForSomeoneElse(address presentReceiver) private {
_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.