Trick or Treat

First Flight #27
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

Use of '_mint()/_transfer()' Instead of '_safeMint()_safeTransfer()' May Lead to Tokens Being Irretrievably Locked

https://github.com/Cyfrin/2024-10-trick-or-treat/blob/9cb3955058cad9dd28a24eb5162a96d759bfa842/src/TrickOrTreat.sol#L81
https://github.com/Cyfrin/2024-10-trick-or-treat/blob/9cb3955058cad9dd28a24eb5162a96d759bfa842/src/TrickOrTreat.sol#L110
https://github.com/Cyfrin/2024-10-trick-or-treat/blob/9cb3955058cad9dd28a24eb5162a96d759bfa842/src/TrickOrTreat.sol#L131

Summary

The SpookySwap contract uses the _mint function to mint NFTs to users without performing safety checks to ensure that the recipient can handle ERC721 tokens. If a user interacts with the contract via a smart contract that does not implement the IERC721Receiver interface, the minted NFT may be permanently locked in the contract, and the user may not be able to access it. The same goes for the _transfer function.

Vulnerability Details

Risk of Token Loss: By using _mint, the contract does not check whether the recipient is capable of receiving ERC721 tokens.
Contracts as Callers: If a smart contract calls trickOrTreat and does not implement IERC721Receiver, the NFT will be sent to it without any mechanism to interact with or transfer the NFT.
User Error: Users might inadvertently interact with the contract through other contracts (e.g., wallets, proxies) that are not ERC721-aware.

Impact

Loss of NFTs: Tokens may become irretrievable if sent to contracts that cannot handle them.
Security Risks: Unintended behavior might occur if tokens are sent to contracts that react unexpectedly upon receiving tokens.
Loss of NFTs: Tokens may become irretrievable if sent to contracts that cannot handle them.

Tools Used

Manual review

Recommendations

Replace all _mint by _safeMint as well as _safeTransfer for _transfer

Updates

Appeal created

bube Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Use of `_mint` instead of `safeMint`

Use of `_transfer` instead of `safeTransfer`

Support

FAQs

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