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
about 2 years ago
inallhonesty Lead Judge about 2 years 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.

Give us feedback!