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

`buyPresent()` function burns from wrong address

0x141414

Summary

The external function buyPresent() burns the SantaTokens of the presentReceiver. This results in a txn fail scenario.

Vulnerability Details

The external function buyPresent() is intended to allow EXTRA_NICE users (addresses holding a SantaToken balance >= 1e18) to buy/mint an NFT for another user, as implied by the address param labeled presentReceiver.

Currently, the buyPresent() function burns 1 (1e18) SantaToken from the presentReceiver address, as shown below.

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

This causes the txn to fail due to an arithmetic underflow error, since it attempts to burn from the presentReceiver's address, whose balance is likely 0.

A PoC test written in forge demonstrates a user being checked as EXTRA_NICE, obtaining SantaTokens, and trying to buy a present for a friend. The txn will fail, resulting in a passed forge test (as indicated by the testFail_ prefix).

function testFail_buyPresentRevert() 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();
assert(santasList.balanceOf(user) == 1);
assert(santaToken.balanceOf(user) == 1e18);
​
// expect fail
santasList.buyPresent(makeAddr("friend"));
}

Impact

Users whose Status is EXTRA_NICE and/or owners of SantaTokens will be unable to mint NFTs for others.

Tools Used

Forge

Recommendations

Change the parameter passed into the i_santaToken.burn() function in buyPresent() from presentReceiver to msg.sender. As shown below:

function buyPresent(address presentReceiver) external {
i_santaToken.burn(msg.sender);
// ...
}
Updates

Lead Judging Commences

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!