GivingThanks
First Flight #28
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unsafe NFT minting, using _mint() instead of _safeMint()

ladiknemcem

01: Summary:

The contract uses mint() when creating the NFT. However, this can result in NFTs being sent to addresses that do not support ERC721 tokens, causing them to become irretrievable. To prevent this, safeMint() should be used instead of _mint() to ensure that the recipient address can safely receive the NFT.

02: Vulnerability Details:

The contract uses _mint() function in this situation:

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
(bool sent,) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
​
_mint(msg.sender, tokenCounter); // right here!
​
// Create metadata for the tokenURI
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
​
tokenCounter += 1;
}

The use of _mint() function directly sends the NFT to the specified address without verifying whether the recipient can receive ERC721 tokens or not. If the recipient is a contract that does not implement the IERC721Receiver interface, the NFT will be locked in that contract and become irretrievable. This could lead to significant loss of assets for the user.

Using *safeMint() instead of *mint() ensures that the recipient's address is either Externally Owned Account or a contract that properly implements the IERC721Receiver interface, preventing this kind of issue.

03: Impact:

Permanent Loss of NFTs: If an NFT is minted to a contract that does not support ERC721 tokens, it could be permanently lost or locked, with no way to retrieve it.

User Financial Loss: Users may lose access to valuable NFTs due to improper handling of the minting process.

Potential Exploitation: An attacker could deliberately target contracts that cannot handle ERC721 tokens to cause NFT loses.

04: Tools Used:

Manual review

05: Recommended Mitigation:

Use safeMint() instead of mint(): replace mint() with safeMint() to ensure that the recipient is capable of receiving ERC721 tokens.

- _mint(msg.sender, tokenCounter);
+ _safeMint(msg.sender, tokenCounter);
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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