GivingThanks

Root Cause and Impact

• Root Cause: The GivingThanks contract uses the _mint() function from the ERC721 standard instead of _safeMint().

• Impact: _mint() does not check whether the recipient address can handle ERC721 tokens. If tokens are minted to a contract that does not support ERC721, the tokens could be locked and inaccessible, leading to loss of assets.

Vulnerability Details

• Use of _mint() Instead of _safeMint():

  function donate(address charity) external payable {

  // ...

  _mint(msg.sender, tokenCounter);

  // ...

  }

  • Issue: _mint() does not perform safety checks on the recipient.

  • Consequence: Tokens may be sent to contracts incapable of handling them, causing them to be locked.

Recommendations

• Use _safeMint() Instead of _mint():

  _safeMint(msg.sender, tokenCounter);

• Benefits of _safeMint():

  • Checks if the recipient address is a contract.

  • If it is a contract, it requires the contract to implement IERC721Receiver.onERC721Received.

  • Prevents tokens from being locked in contracts that cannot handle them.

• Ensure Compliance with ERC721Receiver:

  • If minting to contracts is expected, ensure those contracts implement onERC721Received.

• Considerations:

  • If you are certain that tokens will only be minted to externally owned accounts (EOAs), the risk is minimal.

  • However, using _safeMint() adds an extra layer of security without significant drawbacks.