GivingThanks

Summary

The function updateRegistry() in GivingThanks.sol is critically vulnerable due to the absence of access control, allowing any external actor to modify the address of the registry. This unrestricted access could enable malicious actors to redirect the registry to an arbitrary contract, effectively compromising the contract’s core functions. By pointing to a custom contract, an attacker could override key functions, such as isVerified, and siphon off donor funds under the guise of legitimate registry logic. This type of attack poses a direct and severe risk to donor trust and financial security on the platform.

Vulnerability Details

This function, lacking an access control mechanism, is callable by any user. Without access control, an attacker can supply a malicious contract address as _registry, giving them full control over CharityRegistry logic. This could allow the attacker to:

• Bypass or manipulate verification checks.

• Redirect incoming donations by modifying the behavior of functions like isVerified.

• Ultimately, redirect funds to their own wallet.

For instance, by altering isVerified, an attacker could reroute ether donations to a contract they control, effectively embezzling all donated funds.

Impact

• High Potential for Loss: Direct theft of donor funds, leading to irreversible losses.

• User Trust Compromise: Loss of donor trust if funds are stolen due to platform vulnerabilities.

• Systemic Risk: A lack of access control here could create a systemic vulnerability affecting all interactions with the registry, impacting current and future functionalities dependent on CharityRegistry.

Tools Used

Manual Review: Identified during a thorough inspection of the codebase.

Recommendations

To address this critical issue, implement robust access control measures to restrict access to updateRegistry() as follows:

1. Owner-Only Access Modifier:

   • Introduce an onlyOwner modifier to ensure that only the contract owner can modify the registry address.

   • Example:

     modifier onlyOwner() {

     require(msg.sender == owner, "Access Denied: Only owner can update the registry");

     _;

     }

     function updateRegistry(address _registry) public onlyOwner {

     registry = CharityRegistry(_registry);

     }

     ​

2. Enhanced Contract Verification:

   • Include checks to verify that the address passed to _registry is a contract that adheres to the expected interface for CharityRegistry. Consider implementing EIP-165 (interface identification standard) checks to prevent arbitrary contracts from being registered.

3. Multi-Signature Approval:

   • For additional security, consider requiring multi-signature approval for changes to critical contract addresses, especially for charity-oriented applications handling sensitive transactions.

By implementing these controls, the platform can significantly mitigate the risk of unauthorized registry updates, preserving both the integrity of donor funds and the reputation of the charity platform.