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

Anyone can call `SantasList::buyPresent()` and burn the token of others to collect a present

davide

Summary

The SantasList::buyPresent() should be called only by the people with SantaTokens, so anyone EXTRA_NICE but in reality there is no check in the function.

Vulnerability Details

The function SantasList::buyPresent() is used by the EXTRA_NICE people to buy a present for the NAUGHTY one burning the SantaTokens collected from the SantasList::collectPresent() and receiving a second NFT in exchange, but there is no control on the characteristics of the msg.sender and anyone can call it.

Add the following code in SantasListTest.t.sol:

function testAnyoneCanBuyPresentFromOthers() public {
address attacker = makeAddr("attacker");
​
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);
​
vm.startPrank(user);
santasList.collectPresent();
assertEq(santasList.balanceOf(user), 1);
assertEq(santaToken.balanceOf(user), 1e18);
vm.stopPrank();
​
vm.startPrank(attacker);
santasList.buyPresent(user);
assertEq(santasList.balanceOf(user), 1);
assertEq(santaToken.balanceOf(user), 0);
vm.stopPrank();
}

Impact

Anyone can see the SantaTokens balance of others and watch for token holders calling after that the SantasList::buyPresent(), burning their tokens without limitations and receiving in exchange an NFT as a present.

Tools Used

Manual review.

Recommendations

Make the following changes:

+ error SantasList__NotEnoughSantaTokens();
​
- function buyPresent(address presentReceiver) external {
+ function buyPresent() external {
+ if (i_santaToken.balanceOf(msg.sender) < PURCHASED_PRESENT_COST) {
+ revert SantasList__NotEnoughSantaTokens();
+ }
- i_santaToken.burn(presentReceiver);
+ i_santaToken.burn(msg.sender);
_mintAndIncrement();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
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.