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

Can't buy a NFT for another address through `SantasList::buyPresent(address)` function because it mints the NFT for the caller address (`msg.sender`)

Summary

Can't buy a NFT for another address through SantasList::buyPresent(address) function because it mints the NFT for msg.sender address.

Vulnerability Details

Alice is a EXTRA_NICE person.

Bob is a NAUGHTY person.

If Alice has already collected her present, sent 1e18 Santa Tokens to Bob, and tries to buy Bob a present through SantasList::buyPresent(address) function, 1e18 Santa Tokens of Bob will be burned and Alice will receive the bought present.

Proof of Concept

Apply the following diff:

modified test/unit/SantasListTest.t.sol
@@ -152,4 +152,65 @@ contract SantasListTest is Test {
cmds[1] = string.concat("youve-been-pwned");
cheatCodes.ffi(cmds);
}
+
+ function testFailBuyPresentForNaughty(address randomAddress) public {
+ // `randomAddress` cannot be equal to zero address in order to mint a NFT
+ vm.assume(randomAddress != address(0));
+
+ // Impersonates Santa
+ vm.startPrank(santa);
+
+ // Change `user` status to EXTRA_NICE
+ santasList.checkList(user, SantasList.Status.EXTRA_NICE);
+ santasList.checkTwice(user, SantasList.Status.EXTRA_NICE);
+
+ // Change `randomAddress` status to NAUGHTY
+ santasList.checkList(randomAddress, SantasList.Status.NAUGHTY);
+ santasList.checkTwice(randomAddress, SantasList.Status.NAUGHTY);
+
+ // Stops impersonating Santa
+ vm.stopPrank();
+
+ // Time Travel to Christmas 2023!
+ vm.warp(santasList.CHRISTMAS_2023_BLOCK_TIME() + 1);
+
+ // Impersonates `user`
+ vm.startPrank(user);
+
+ // Collect present
+ santasList.collectPresent();
+
+ // Amount of Santa Token received
+ uint256 tokenBalance = santaToken.balanceOf(user);
+
+ // Transfer `user` Santa Tokens to `randomAddress`
+ santaToken.transfer(randomAddress, tokenBalance);
+
+ // Stops impersonating `user`
+ vm.stopPrank();
+
+ // Impersonates `randomAddress`
+ vm.startPrank(randomAddress);
+
+ // Approve `santasList` to burn `randomAddress` Santa Tokens
+ santaToken.approve(address(santasList), tokenBalance);
+
+ // Stops impersonating `randomAddress`
+ vm.stopPrank();
+
+ // Impersonates `user` again
+ vm.startPrank(user);
+
+ // Buy `randomAddress` a nice present
+ santasList.buyPresent(randomAddress);
+
+ // Stops impersonating `user`
+ vm.stopPrank();
+
+ // Assert if `randomAddress` Santa Token balance has been burned
+ assertEq(santaToken.balanceOf(randomAddress), 0);
+
+ // Assert if `randomAddress` has received the bought a NFT
+ assertEq(santasList.balanceOf(randomAddress), 1);
+ }
}

And run the testFailBuyPresentForNaughty test:

forge test --match-test testFailBuyPresentForNaughty

Impact

The target address - presentReceiver argument of SantasList::buyPresent(address) function - will have 1e18 Santa Tokens burned and will not receive the NFT.

Tools Used

  • Manual Review

  • GNU Emacs (solidity-mode + magit)

  • Foundry test

Recommendations

Inside SantasList::buyPresent(address) replace _mintAndIncrement() with _safeMint(presentReceiver, s_tokenCounter++):

modified src/SantasList.sol
@@ -171,7 +171,7 @@ contract SantasList is ERC721, TokenUri {
*/
function buyPresent(address presentReceiver) external {
i_santaToken.burn(presentReceiver);
- _mintAndIncrement();
+ _safeMint(presentReceiver, s_tokenCounter++);
}
/*//////////////////////////////////////////////////////////////
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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