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

Reentrancy Vulnerability in the GivingThanks::donate function

ghufranhassan1

Summary


The donate function is vulnerable to reentrancy attacks due to the external call to charity.call without adhering to CEI pattern.

Vulnerability Details


The donate function transfers Ether to a charity using charity.call{value: msg.value}(""). If the charity contract has a fallback or receive function, it could potentially re-enter the donate function before the state changes (like _mint and tokenCounter increment).

Impact

Without protection, a malicious charity contract could repeatedly call donate, draining funds and minting multiple tokens.

Tools Used

Manual Review

Recommendations

Apply CEI pattern

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