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 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.