GivingThanks

Root Cause and Impact

• Root Cause: The donate function violates the Checks-Effects-Interactions (CEI) pattern by making an external call before updating internal state.

• Impact: A malicious charity contract could exploit this to perform a reentrancy attack, potentially causing multiple unintended token mints or disrupting the donation process.

Vulnerability Details

• donate Function Code:

  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);

  // ...

  }

  • Issue: External call to charity.call occurs before _mint.

  • Consequence: If charity is a malicious contract, it could re-enter the donate function.

Recommendations

• Reorder Operations Following CEI Pattern:

  function donate(address charity) public payable {

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

  _mint(msg.sender, tokenCounter);

  // Update token URI and counter

  // ...

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

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

  }

• Use Reentrancy Guards:

  • Implement OpenZeppelin's ReentrancyGuard:

    contract GivingThanks is ERC721URIStorage, ReentrancyGuard {

    // ...

    function donate(address charity) public payable nonReentrant {

    // Function code

    }

    }

• Validate the charity Address:

  • Ensure charity is not a contract with fallback functions that could exploit reentrancy.