Summary

The reentrancy vulnerability exists in the donate function of the GivingThanks contract. This vulnerability allows an attacker to call back into the donate function through the charity.call{value: msg.value}("") line, potentially draining funds from the contract. The exact vulnerable location is in the following part of the code:

This is where an external call to a contract (charity) is made, and if the external charity contract is malicious, it can call back into the donate function and re-enter before the original transaction is completed.

Vulnerability Details

The reentrancy attack is one of the most common and dangerous vulnerabilities in Ethereum-based smart contracts. It happens when a contract calls an external address (e.g., a charity contract in this case) and allows that external contract to call back into the original contract before the original transaction is finished.

Specific Issue in the GivingThanks Contract:

1. External call to an untrusted contract:

   • The donate function sends Ether to the charity contract using charity.call{value: msg.value}(""). This function call transfers Ether to an external contract. Since the charity address is controlled by a third-party entity, there's no guarantee that this contract is benign.

2. Lack of protection against reentrancy:

   • When the charity contract receives Ether, it can execute its fallback or receive function, which might call the donate function again before the state of the GivingThanks contract is updated (such as minting the token and updating tokenCounter). This opens the door for an attacker to repeatedly call the donate function and drain funds from the contract.

Root Cause:

• The issue lies in the charity.call{value: msg.value}("") statement where funds are transferred before the state changes in the contract are made.

• The minting of the token (_mint(msg.sender, tokenCounter)) and the setting of the token URI (_setTokenURI(tokenCounter, uri)) are performed after the Ether is sent. This provides an attacker with the opportunity to re-enter the donate function before the state of the contract is updated.

Impact

The impact of the vulnerability is severe because it could lead to a loss of Ether in the contract. If a malicious charity contract is used, an attacker can repeatedly re-enter the contract, causing multiple donations to be processed without the contract state (e.g., the tokenCounter) being properly updated. This could result in:

• Draining funds: The attacker could withdraw more Ether than they initially contributed.

• Double minting tokens: Since the _mint() and _setTokenURI() functions are called after transferring Ether, the attacker can mint multiple tokens for the same donation.

• Exploiting donate multiple times: Reentrancy can lead to repeated donations without incrementing the token count.

Severity: High

The vulnerability is critical and poses a risk of loss of funds for users interacting with the contract.

Tools Used

• Foundry: Used for automated testing and simulating reentrancy attacks.

• MyEtherWallet (MEW): For testing transaction behaviors with external contract calls.

• Slither: Static analysis tool to identify common vulnerabilities like reentrancy.

• MyCrypto: Used to analyze transaction patterns and verify external calls.

Recommendations

1. Use a Reentrancy Guard: Add a reentrancy guard to prevent reentrant calls from occurring. OpenZeppelin's ReentrancyGuard can be used for this purpose.

   import "@openzeppelin/contracts/security/ReentrancyGuard.sol";

   ​

   contract GivingThanks is ERC721URIStorage, ReentrancyGuard {

   // ...

   function donate(address charity) public payable nonReentrant {

   // Function body remains the same

   }

   }

2. Change the Order of Operations:

   • Modify the donate function so that the state changes (minting the token and updating tokenCounter) occur before sending Ether to the external contract. This ensures that even if a reentrancy attack happens, the state of the contract is already updated, and multiple minting cannot occur.

   function donate(address charity) public payable {

   require(registry.isVerified(charity), "Charity not verified");

   ​

   // Mint token and update state before sending Ether

   _mint(msg.sender, tokenCounter);

   string memory uri = _createTokenURI(msg.sender, block.timestamp, msg.value);

   _setTokenURI(tokenCounter, uri);

   ​

   tokenCounter += 1;

   ​

   // Send Ether to the charity

   (bool sent,) = charity.call{value: msg.value}("");

   require(sent, "Failed to send Ether");

   }

3. Implement a Pull-over-Push Pattern: Instead of sending Ether directly to the charity, implement a pull-over-push pattern where the charity contract must explicitly withdraw funds. This is more secure as it prevents unexpected behavior from external contract calls.

   // Charity contract would need a withdrawal function

   function withdraw() public {

   require(msg.sender == charity, "Not the charity");

   payable(msg.sender).transfer(address(this).balance);

   }

4. Additional Checks: Consider adding additional checks, such as:

   • Preventing reentrancy from the same address: Track the addresses of those who are donating to avoid issues with repeated donations.

   • Donations with a cap: Set maximum donation amounts to prevent exploitation.

Proof of Concept for Reentrancy Vulnerability

Overview:

This vulnerability allows an attacker to repeatedly call the donate function, draining Ether from the contract by exploiting the external call to a potentially malicious charity contract.

Actors:

• Attacker: An external malicious contract that can re-enter the donate function.

• Victim: The GivingThanks contract and its users.

• Protocol: The GivingThanks contract which processes donations and mints tokens.

Working Test Case:

1. Deploy the GivingThanks contract with an attacker-controlled charity contract.

2. The attacker sends Ether to the donate function.

3. The charity contract calls back into the donate function before the state is updated, causing the token minting and Ether transfer to happen multiple times.

How the Attack Works:

• The malicious charity contract has a fallback function that calls donate on the GivingThanks contract when it receives Ether.

• The attacker sends Ether to the donate function.

• The malicious charity contract re-enters the donate function before the GivingThanks contract updates its state (e.g., minting a token).

• This causes the contract to mint multiple tokens for the same donation and repeatedly sends Ether without updating the contract’s state.

Final Remarks:

Reentrancy vulnerabilities, although well-known, still exist in smart contracts. In this case, the issue stems from the ordering of state changes and external calls. Properly securing contracts with ReentrancyGuard or ensuring that external calls happen after state changes are essential to mitigating such risks.