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

Contract Initialization Bug

bbash

Summary

The registry variable should be initialized with the _registry parameter passed to the constructor, rather than msg.sender.

Vulnerability Details

Currently, the registry variable is mistakenly initialized with msg.sender, which can cause unintended functionality by linking to the deploying address instead of the intended CharityRegistry address.

https://github.com/Cyfrin/2024-11-giving-thanks/blob/main/src/GivingThanks.sol#L15-L19

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

Impact

Misinitialization can prevent the contract from correctly referencing the designated CharityRegistry instance, potentially disrupting the verification checks for charities and leading to improper access control.

Tools Used

Manual Review

Recommendations

Ensure registry is initialized with _registry to accurately set the intended CharityRegistry contract:

registry = CharityRegistry(_registry);
Updates

Lead Judging Commences

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