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

```SantaList:buyPresent()``` can't buy and send the Present directly to someone else

kiteweb3

Summary

SantaList:buyPresent() can't buy and send the Present directly to someone else. When a user invokes SantaList::buyPresent(), the functionality is restricted to acquiring a present exclusively for themselves, not for someone else. The buyPresent() function necessitates providing the presentReceiver address as the destination for the Present NFT. While the user incurs a cost of 1 SantaToken for the purchase, it is important to note that the function attempts to burn the SantaToken from the presentReceiver address, not the msg.sender. This reverts because the user may not have the SantaToken and in case he/she hasn't authorized the SantasList contract to spend their SantaTokens. The only way to buy a Present for someone else, is to buy the Present for themself and then transfer to the Present recipient.

Vulnerability Details

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

Impact

//The Present NFT can't be bought for someone else
function testCantBuyAPresentforSomeoneElse() public {
//The user2 is EXTRA_NICE and collects the present and the SantaToken
vm.startPrank(santa);
santasList.checkList(user2, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user2, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
​
vm.startPrank(user2);
santasList.collectPresent();
assertEq(santasList.balanceOf(user2), 1);
assertEq(santaToken.balanceOf(user2), 1e18);
vm.stopPrank();
​
//The user2 tries to buy a present for the user
vm.startPrank(user2);
santaToken.approve(address(santasList), 1e18);
​
vm.expectRevert();
santasList.buyPresent(user);
​
vm.stopPrank();
}

Tools Used

Manual review

Recommendations

Evaluate to modify the buyPresent() function for burning the msg.sender SantaToken amount and safeTransferFrom() the Present to the presentReceiver.

function buyPresent(address presentReceiver) external {
- i_santaToken.burn(presentReceiver);
+ i_santaToken.burn(msg.sender);
_mintAndIncrement();
+ safeTransferFrom(msg.sender, presentReceiver, s_tokenCounter - 1);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
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.

buyPresent should send to presentReceiver

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.