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

Incorrect Assignment of CharityRegistry Address

bryanconquer

Summary

In the GivingThanks smart contract, there is an issue in the constructor where the registry variable, intended to store the address of the CharityRegistry contract, is incorrectly assigned. Instead of using the _registry parameter passed during deployment, the contract mistakenly assigns msg.sender to the registry variable. This results in the contract deployer's address being cast as the CharityRegistry contract.

Vulnerability Details

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

Impact

Incorrect Registry Assignment: The contract will treat the deployer's address as the CharityRegistry, which is not the intended behavior. This can cause all functions dependent on the registry variable to fail or produce incorrect results.

  • Potential Security Risks: Since the deployer's address is treated as the registry, the contract could behave unpredictably, especially if the deployer is not a legitimate charity registry contract.

Tools Used

manula review

Recommendations

the constructor should assign the _registry parameter to the registry variable.

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
registry = CharityRegistry(_registry); // Correct assignment
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.