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

NFT possibly getting stuck

safiullahmubashir08

Summary

https://github.com/Cyfrin/2024-11-giving-thanks/blob/304812abfc16df934249ecd4cd8dea38568a625d/src/GivingThanks.sol#L26

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);
​
// Create metadata for the tokenURI
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
​
tokenCounter += 1;
}

Vulnerability Details

Impact

The _mint function does not implement checkOnERC721Received which lets us know if a smart contract knows how to handle an NFT

Tools Used

Manual review

Recommendations

The _mint function needs to be replaced with _safeMint which will cause the transaction to fail if the smart contract cannot support NFTs

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);
​
// Create metadata for the tokenURI
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(tokenCounter, uri);
​
tokenCounter += 1
}

But here are a few more consideration the _safeMint function will make an external call to the smart contract to call checkOnERC721Received which could be used for a rentrancy attack. Right now since the contract holds no balance that attack would be meaningless

However if the protocol plans to take fee on transfers, this could be used as an attack vector to drain the smart contract so a rentrancy guard would be usefull here

Updates

Lead Judging Commences

n0kto Lead Judge 12 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.