GivingThanks

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

Reentrancy Vulnerability found in `GivingThanks::donate()`

Description: The GivingThanks::donate function is vulnerable to reentrancy attacks due to state changes after external calls.

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);
// ... rest of the function
}

Impact:

  • Multiple NFTs could be minted for a single donation

  • Token counter could be manipulated

  • Potential drainage of contract funds

Proof of Concept:

contract AttackingCharity {
GivingThanks public target;
receive() external payable {
if (address(target).balance >= 1 ether) {
target.donate{value: 1 ether}(address(this));
}
}
}

Recommended Mitigation: The donate() function should be like this:

function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
uint256 currentToken = tokenCounter++;
_mint(msg.sender, currentToken);
string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);
_setTokenURI(currentToken, uri);
(bool sent,) = charity.call{value: msg.value}("");
require(sent, "Failed to send Ether");
}
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.