GivingThanks

Summary

The GivingThanks::donate function is intended to allow users to donate Ether to a verified charity, mint a token, and create metadata for the token URI. However, the function contains a reentrancy vulnerability. This vulnerability arises because the function updates the contract's internal state (minting tokens and updating tokenCounter) after it sends Ether to the charity. This can allow malicious contracts to exploit the order of operations and perform reentrancy attacks.

Vulnerability Details

In the current implementation, the contract sends Ether to the charity first, and then performs state-changing actions such as:

• Minting a token (_mint(msg.sender, tokenCounter)).

• Creating and setting the token URI (_createTokenURI and _setTokenURI).

• Incrementing the tokenCounter.

By updating the state after the Ether transfer, there is an opportunity for a reentrancy attack if a malicious charity contract is involved. Here's how the attack could occur:

• A malicious contract is registered as the charity.

• The malicious contract receives Ether via call{value: msg.value}("").

• Upon receiving the Ether, the malicious contract could call back into the donate() function before the state has been updated (i.e., before the token is minted and tokenCounter is incremented).

• This allows the malicious contract to potentially re-enter the donate() function multiple times, causing multiple donations to be processed before the state is correctly updated.

• This type of attack could result in the following issues:

Minting more tokens than intended.
Causing unexpected state changes, such as incorrect token assignments or token count.

Impact

• Reentrancy Attack: Malicious contracts could repeatedly call the donate() function, causing state inconsistencies, such as minting extra tokens.

• Token Overminting: An attacker could mint more tokens than expected, either draining the contract’s intended resources or causing incorrect assignments.

• Loss of Funds: If state changes depend on certain logic after receiving the donation (e.g., token minting), a reentrancy attack could interfere with this process and result in losses or unauthorized access to funds.

Tools Used

Manual Audit: Reviewing the contract flow and checking the order of operations (state changes before external calls) can highlight potential vulnerabilities.

Recommendations

• Follow Checks-Effects-Interactions Pattern: Update the internal state (e.g., minting the token, updating tokenCounter) before transferring Ether. This prevents external contracts from being able to re-enter the function and manipulate the state before it's updated.

• State Changes First: Move the minting and token URI creation logic above the call statement. This ensures that state updates occur before any external interaction, preventing malicious reentrant calls.