GivingThanks

Root Cause and Impact

• Root Cause: The updateRegistry function in the GivingThanks contract lacks access control modifiers, allowing anyone to change the registry address. Similarly, the registerCharity function in the CharityRegistry contract permits any user to register any address as a charity without any verification or restrictions.

• Impact: Malicious actors can update the registry to point to a fraudulent contract or register unauthorized addresses as charities. This could lead to misdirection of donations, fraud, and loss of user trust.

Vulnerability Details

• updateRegistry Function in GivingThanks Contract:

  function updateRegistry(address _registry) public {

  registry = CharityRegistry(_registry);

  }

  • Issue: No access control; any user can call this function.

  • Consequence: Attackers can redirect the registry to a malicious contract.

• registerCharity Function in CharityRegistry Contract:

  function registerCharity(address charity) public {

  registeredCharities[charity] = true;

  }

  • Issue: No restrictions on who can register a charity.

  • Consequence: Unauthorized or fraudulent addresses can be registered as charities.

Recommendations

• Implement Access Control:

  • Use OpenZeppelin's Ownable contract to restrict access to sensitive functions.

  • Add the onlyOwner modifier to functions that should be restricted.

  // Inherit from Ownable

  contract GivingThanks is ERC721URIStorage, Ownable {

  // ...

  function updateRegistry(address _registry) public onlyOwner {

  registry = CharityRegistry(_registry);

  }

  }
  contract CharityRegistry {

  // ...

  function registerCharity(address charity) public onlyAdmin {

  registeredCharities[charity] = true;

  }

  modifier onlyAdmin() {

  require(msg.sender == admin, "Only admin can perform this action");

  _;

  }

  }

• Add Proper Verification Mechanisms:

  • Introduce a process to verify charities before registration.

  • Ensure only trusted entities can register and verify charities.