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

[H-01] Reentrancy attack in GivingThanks::donate() function, leads to duplicate NFT minting and funds drained

yuza101

Summary

The donate function of the GivingThanks contract is vulnerable to a reentrancy attack due to making an external call to a charity address using call before updating its internal state tokenCounter. This allows a malicious contract to re-enter the donate function multiple times, minting multiple NFTs with the same token ID, violating the ERC721 standard for unique tokens.

Vulnerability Details

GivingThanks.sol(#21-33):

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;
}

The external call to charity address executes before updating the tokenCounter.

  • Attacker creates a malicious contract with fallback() function. Takes the contract's address and pass into the charity. Require statement checks if the charity is verified.

  • The call interacts with the charity address which could be a malicious contract address with a fallback function.

  • The fallback function will re-enter the donate function, allowing the charity contract to call donate multiple times before the state tokenCounter is updated.

  • The _mint function is called multiple times with the same tokenCounter value, creating multiple NFTs with the same/duplicated token ID.

Impact

  • Multiple NFTs being minted with the same tokenCounter, violating the uniqueness property of ERC721 tokens.

  • Drain funds due to the fallback() function repeatedly executes the donation process.

Tools Used

Slither + Manual Code Review

Recommendations

  • Update the state before the external call.

  • Utilize OpenZeppelin's ReentrancyGuard library to prevent Reentrancty attack.

Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-donate-reentrancy-multiple-NFT-minted

Impact: High, one charity can reenter the donate function with the same ETH provided and mint several NFT. Likelyhood: Low, any malicious charity can do it but Admin is trusted and should verify the charity contract before "verifying" it.

Support

FAQs

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