GivingThanks

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

Bug Report: Constructor Conflict in GivingThanks.sol and CharityRegistry.sol

Summary:

A deployment conflict occurs due to a mismatch in the expected address for the CharityRegistry contract instance in GivingThanks.sol. This prevents the GivingThanks contract from deploying correctly, as the _registry parameter is not utilized as intended in the constructor.

Vulnerability Details:

In GivingThanks.sol, the constructor receives an address parameter _registry intended to reference an existing CharityRegistry contract instance. However, the _registry address is not correctly utilized in the contract, which results in a failure during deployment. This makes the GivingThanks contract unusable as it cannot be deployed to the blockchain without addressing this issue.

Impact

High Impact: The deployment failure blocks the GivingThanks contract from functioning, preventing any interactions on-chain. This issue effectively halts the contract’s deployment and intended operation.

Tools Used

Tools Used

  • Manual code review

  • Foundry (for compilation and testing)

forge compile

Compiler Output

Compiler run successful with warnings:
Warning (5667): Unused function parameter. Remove or comment out the variable name to silence this warning.
--> src/GivingThanks.sol:15:17:
|
15 | constructor(address _registry) ERC721("DonationReceipt", "DRC") {
| ^^^^^^^^^^^^^^

Recommendations

To resolve the issue, modify the constructor in GivingThanks.sol to properly utilize the _registry parameter, as shown below:

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

This change will correctly assign registry to an instance of the CharityRegistry contract, allowing the GivingThankscontract to interact with it as intended.

Updates

Lead Judging Commences

n0kto Lead Judge 9 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.