Summary

The contract GivingThanks has a critical access control vulnerability in the updateRegistry function. The function allows anyone to update the address of the CharityRegistry contract, which could lead to an attacker changing the registry to a malicious contract, causing funds to be sent to a malicious charity.

Location of Vulnerability:

The vulnerability is located in the updateRegistry function:

This function does not have any restrictions on who can call it, meaning that any address (including malicious ones) can change the address of the CharityRegistry contract.

Vulnerability Details

The updateRegistry function is intended to allow the owner or an authorized party to update the address of the CharityRegistry contract. However, it does not include any access control mechanisms, such as the onlyOwner modifier from OpenZeppelin's Ownable contract, to restrict who can call it.

Exploit Scenario:

An attacker can exploit this vulnerability by calling the updateRegistry function and changing the registry address to a malicious contract. Once the registry is updated, the attacker could either:

• Use a malicious CharityRegistry contract that marks any charity as verified, allowing them to send funds to their own account.

• Use a contract that could manipulate or hijack the charity verification process, allowing an attacker to redirect donations to a different address.

Since there is no restriction on who can call updateRegistry, any external address can potentially change the registry address and bypass the verification process.

Root Cause:

• The absence of proper access control in the updateRegistry function allows anyone to modify the registry address.

• No checks are in place to ensure only the contract owner or authorized personnel can update the registry contract, leading to potential abuse.

Consequences:

• Funds redirect: Attackers can change the registry to a malicious contract, redirecting donations to themselves or any address they control.

• Untrusted charity verification: An attacker could bypass the verification mechanism by setting up their own fraudulent registry contract, making it appear that unverified charities are legitimate.

• Undermines the contract’s integrity: If an attacker gains control over the registry, they can manipulate or sabotage the entire donation process, leading to a loss of trust in the contract.

Impact

• Redirected Donations: By changing the registry, funds could be sent to a malicious charity address under the guise of a verified charity, causing financial loss.

• Increased Attack Surface: The lack of access control introduces a new attack vector, which could be exploited by anyone interacting with the contract.

• Loss of Trust: If the contract is exploited, users may lose confidence in the GivingThanks platform, which could damage its reputation and usage.

Tools Used

• Slither: Static analysis tool to identify vulnerabilities like unauthorized access and insecure function calls.

• MyEtherWallet (MEW): Used to manually interact with the contract to simulate unauthorized function calls.

• Foundry: Used for testing and creating scenarios to demonstrate the vulnerability in action.

• MyCrypto: Used for analyzing contract functions and testing access control.

Recommendations

1. Add Access Control:
   Use the Ownable contract from OpenZeppelin or a similar mechanism to ensure that only the contract owner (or other authorized addresses) can call the updateRegistry function.

   Example:

   import "@openzeppelin/contracts/access/Ownable.sol";

   ​

   contract GivingThanks is ERC721URIStorage, Ownable {

   // Now the owner can call updateRegistry, but no one else

   function updateRegistry(address _registry) public onlyOwner {

   registry = CharityRegistry(_registry);

   }

   }

2. Restrict Updates to Trusted Parties:
   In addition to restricting the function to only the owner, it might also be beneficial to implement multi-signature or role-based access control (e.g., using OpenZeppelin's AccessControl), allowing a group of trusted parties to make such updates.

   Example:

   import "@openzeppelin/contracts/access/AccessControl.sol";

   ​

   contract GivingThanks is ERC721URIStorage, AccessControl {

   bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");

   ​

   constructor(address _registry) ERC721("DonationReceipt", "DRC") {

   _setupRole(DEFAULT_ADMIN_ROLE, msg.sender); // Grant admin role to the contract deployer

   }

   ​

   function updateRegistry(address _registry) public onlyRole(ADMIN_ROLE) {

   registry = CharityRegistry(_registry);

   }

   }

3. Implement an Emergency Pause Function:
   Consider adding an emergency pause function (using OpenZeppelin's Pausable contract) to temporarily halt contract operations if suspicious activity is detected.

4. Audit External Contracts:
   Always ensure that any external contract (like the CharityRegistry contract) is thoroughly audited before it is linked to the GivingThanks contract. In addition, the contract could verify if the provided registry address is valid before updating it.

Proof of Concept for Lack of Access Control

Overview:

An attacker can exploit the lack of access control in the updateRegistry function to change the registry address to a malicious contract, redirecting funds to themselves or controlling the charity verification process.

Actors:

• Attacker: The attacker is anyone who interacts with the updateRegistry function without restrictions and changes the registry to a malicious contract.

• Victim: The victim is any donor who sends funds, trusting that the charity registry is legitimate and secure.

• Protocol: The GivingThanks contract and the CharityRegistry contract.

Working Test Case:

1. Deploy a Malicious Registry Contract:
   The attacker deploys a fraudulent CharityRegistry contract that always returns true for charity verification, making any charity appear verified.

   // Malicious CharityRegistry contract

   pragma solidity ^0.8.0;

   ​

   contract MaliciousCharityRegistry {

   function isVerified(address charity) public pure returns (bool) {

   return true; // Always return true, no matter what charity is passed

   }

   }

2. Exploit the updateRegistry Function:
   The attacker interacts with the GivingThanks contract and calls the updateRegistry function, changing the registry address to the malicious contract.

   // Attacker updates the registry to the malicious contract

   givingThanks.updateRegistry(address(maliciousRegistry));

3. Redirect Donations:
   The attacker then calls the donate function to send Ether to the contract, and the GivingThanks contract now recognizes any charity address as verified because the registry always returns true. The funds will be sent to the attacker's controlled charity address.

4. Observe the Impact:
   The attacker can now collect donations and send them to their own address, or they could use the same approach to hijack the entire charity verification system.

Conclusion:

The vulnerability in the updateRegistry function allows an attacker to hijack the charity verification process by changing the registry to a malicious contract. This is a high-risk vulnerability because it allows an attacker to reroute donations to themselves or manipulate the contract's logic to perform malicious activities. Adding access control and restricting who can modify critical contract parameters will mitigate this risk.