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

Reentrancy Vulnerability in donate() Function

sauronsol

Summary

The donate() function in GivingThanks contract is vulnerable to reentrancy attacks, allowing malicious contracts to mint multiple NFTs while only paying once.

Vulnerability Details

The vulnerability exists because the contract:

  • Sends ETH via low-level call

  • Performs state changes after the external call

  • Has no reentrancy protection

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
(bool sent,) = charity.call{value: msg.value}(""); // [!] External call
require(sent, "Failed to send Ether");
_mint(msg.sender, tokenCounter); // [!] State change
// ...state changes continue...
}

Impact

High severity:

  • Attacker can mint multiple NFTs for the price of one

  • Contract state can be manipulated

  • Donation accounting becomes incorrect

  • Possible DOS if token limit exists

Tools Used

  • Manual code review

  • Foundry testing framework

  • Custom reentrancy attack contract

  • Test showing successful exploitation

Recommendations

  1. Implement Checks-Effects-Interactions pattern:

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
// Effects first
uint256 currentTokenId = tokenCounter;
tokenCounter += 1;
_mint(msg.sender, currentTokenId);
_setTokenURI(currentTokenId, _createTokenURI(...));
// Interactions last
(bool sent,) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
}

  1. Alternative: Use OpenZeppelin's ReentrancyGuard:

contract GivingThanks is ERC721URIStorage, ReentrancyGuard {
function donate(address charity) public payable nonReentrant {
// Original implementation
}
}
Updates

Lead Judging Commences

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