Summary
Using ERC721::_mint() can mint ERC721 tokens to addresses which don't support ERC721 tokens. This means if a donor is calling GivingThanks::donate from an address which does not support the ERC721 standard, they will not receive a receipt.
Vulnerability Details
ERC721::_mint() does not perform any checks to ensure that the recipient address can handle ERC721 tokens. This is fine in most cases, but issues can arise if the GivingThanks::donate functions is called from a contract that is not prepared to handle ERC721 tokens. Thus, the receipt issued by GivingThanks will be lost because it cannot be retreived from the contract that received the token.
Proof of Concept
Donor has a contract that does not support ERC721 tokens
Donor donates to the charity via contract
Charity contract mints a token using ERC721::_mint()
Transaction does not revert even though the ERC721 token receipt cannot be retreived from the contract
Donating to the charity via contract using ERC721::_safeMint() reverts when donor donates via contract
Code:
The following test demonstrates how using ERC721::_mint() does not revert if the recipient address does not support ERC721 tokens, but using ERC721::_safeMint() does.
function testMintVsSafeMint() public {
uint256 donationAmount = 1 ether;
DumbContract dumbContract = new DumbContract();
vm.deal(address(dumbContract), 10 ether);
vm.prank(address(dumbContract));
charityContract.donate{value: donationAmount}(charity);
GivingThanksSafeMint charityContractSafeMint = new GivingThanksSafeMint(address(registryContract));
vm.prank(address(dumbContract));
vm.expectRevert(abi.encodeWithSelector(IERC721Errors.ERC721InvalidReceiver.selector, address(dumbContract)));
charityContractSafeMint.donate{value: donationAmount}(charity);
}
An adapted version of GivingThanks.sol was used for this test to implement the _safeMintfunction for demo purposes.
pragma solidity ^0.8.0;
import "src/CharityRegistry.sol";
import "@openzeppelin/contracts/token/ERC721/extensions/ERC721URIStorage.sol";
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/utils/Strings.sol";
import "@openzeppelin/contracts/utils/Base64.sol";
contract GivingThanksSafeMint is ERC721URIStorage {
CharityRegistry public registry;
uint256 public tokenCounter;
address public owner;
constructor(address _registry) ERC721("DonationReceipt", "DRC") {
registry = CharityRegistry(_registry);
owner = msg.sender;
tokenCounter = 0;
}
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");
_safeMint(msg.sender, tokenCounter);
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
tokenCounter += 1;
}
function _createTokenURI(address donor, uint256 date, uint256 amount) internal pure returns (string memory) {
string memory json = string(
abi.encodePacked(
'{"donor":"',
Strings.toHexString(uint160(donor), 20),
'","date":"',
Strings.toString(date),
'","amount":"',
Strings.toString(amount),
'"}'
)
);
string memory base64Json = Base64.encode(bytes(json));
return string(abi.encodePacked("data:application/json;base64,", base64Json));
}
function updateRegistry(address _registry) public {
registry = CharityRegistry(_registry);
}
}
Impact
When a donor donates via contract that does not support ERC721 tokens, the receipt will be lost and cannot be retreived from the contract. If a receipt is needed, this is an issue as receipts cannot be issued after the donation completed. However, this does not cause any loss of funds and is not a security issue.
Tools Used
Foundry, Aderyn, manual review
Recommendations
Use _safeMint() instead of _mint() for ERC721.
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");
+ _safeMint(msg.sender, tokenCounter);
- _mint(msg.sender, tokenCounter);
// Create metadata for the tokenURI
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
tokenCounter += 1;
}
