GivingThanks

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

Incorrect Initialization of registry in GivingThanks Constructor

Summary

In the constructor GivingThanks contract, parameter _registry is not used in the body of the constructor and msg.sender is used instead.

Vulnerability Details

The constructor of GivingThanks uses msg.sender instead of _registry to set the registry address, which likely results in an invalid reference to CharityRegistry.

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
registry = CharityRegistry(msg.sender);
owner = msg.sender;
tokenCounter = 0;
}
  • Using CharityRegistry(msg.sender) this way can lead to unintended behavior if msg.sender is not actually a CharityRegistry contract address.

  • Any function calls to registry (e.g., registry.isVerified(charity)) will fail if msg.sender is not a deployed instance of CharityRegistry

Impact:

high as This prevents GivingThanks from interacting with the intended CharityRegistry instance, breaking the charity verification functionality in the donate function.

Probability:

hight as This will happen in the constructor of GivingThanks

Tools Used

Foundry, Slither, Aderyn

Recommendations

Change the CharityRegistry(msg.sender) to use the input parameter instead of msg.sender

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

Lead Judging Commences

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