GivingThanks

First Flight #28
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: medium
Valid

M-04 Incorrect Initialization of `registry` in `GivingThanks` Constructor Causes Malfunction

Summary

The GivingThanks contract incorrectly initializes the registry variable in its constructor by assigning it to msg.sender instead of using the _registry parameter provided. This error results in the registry variable pointing to the deployer's address rather than the actual CharityRegistry contract. Consequently, any calls to registry.isVerified() within the GivingThanks contract do not function as intended, leading to failures in the donation process.

Vulnerability Details

Affected Code: GivingThanks:constructor line 16

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
registry = CharityRegistry(msg.sender);
owner = msg.sender;
tokenCounter = 0;
}

Issue:

  • The constructor is intended to set the registry variable to the address of the deployed CharityRegistry contract passed as _registry.

  • Instead, it incorrectly assigns registry = CharityRegistry(msg.sender);, which sets registry to the address of the deployer (msg.sender) of the GivingThanks contract.

  • This misassignment means that any interaction with registry within the contract refers to an incorrect address, causing functions like registry.isVerified() to fail or return incorrect results.

Proof of Concept:

When deploying the GivingThanks contract and attempting to use its donate function, the contract will fail to verify charities correctly because registry does not point to the actual CharityRegistry contract.

Impact

  • Functional Failure: The donate function relies on registry.isVerified(charity) to validate charities. With registry incorrectly set, this validation fails, and donations cannot be processed.

  • User Frustration: Donors attempting to contribute will encounter transaction failures, leading to a poor user experience.

  • Operational Disruption: The core functionality of the GivingThanks contract is compromised, hindering the platform's purpose.

Tools Used

  • Manual Code Review: Identified the incorrect assignment in the constructor.

  • Testing Frameworks: Simulated contract deployment and interactions to observe the malfunction.

  • Solidity Compiler: Checked for warnings and errors during compilation.

Recommendations

  • Correct the Constructor Initialization:

    Update the constructor to properly assign the _registry parameter to the registry variable:

    constructor(address _registry) ERC721("DonationReceipt", "DRC") {
    - registry = CharityRegistry(msg.sender);
    + registry = CharityRegistry(_registry);
    owner = msg.sender;
    tokenCounter = 0;
    }

By making this correction, the GivingThanks contract will function properly, allowing it to interact with the CharityRegistry contract as intended. This ensures that only verified charities can receive donations, maintaining the integrity and purpose of the platform.

Updates

Lead Judging Commences

n0kto Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-bad-registry-set-at-construction

Likelyhood: High, the parameter is not well used and won't be set. Impact: Low, can be changed with the setter and no one will be able to donate to malicious charity.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.