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

Donate function is susceptible to Reentrancy manipulation.

sala1

Summary

GivingThanks::donate funciton is susceptible to Reentrancy attack since the change of the state variable comes after the external call (GivingThanks::donate function, see below code line 4). State varialbe changes below code line 14.

Vulnerability Details + Recommendations

// see comments @audit
function donate(address charity) public payable {
require(registry.isVerified(charity), "Charity not verified");
(bool sent,) = charity.call{value: msg.value}(""); // @audit recommended changes goes above this line.
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);
// @audit recommended change. Below line should go before line 4 (the call)
tokenCounter += 1;
}

Impact

Tools Used

Foundry test

Manual review

Recommendations

Either of the following solutions should work.

  1. Use Checks-Effects-Interactions Pattern indicated by the @comment in code above by moving around lines of code.

  2. Or use openZeppelin nonReentrant modifier with the function: GivingThanks::donate. This means doing the setup such as [GivingThanks::constructor is ReentrancyGuard, then adding the modifier to the donate function].

​
Updates

Lead Judging Commences

n0kto Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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