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

Unused variable in the constructor and invalid initialisation of CharityRegistry.

faisalali19

Summary

The constructor includes an unused variable _registry, and the value assigned to registry might be invalid if it points to an externally owned account (EOA) or a contract that is not an instance of CharityRegistry

Vulnerability Details

registry = CharityRegistry(msg.sender); is assigned an invalid address if the msg.sender is an EOA account or if its a contract which is not an instance of the CharityRegistry. Also _registry is an unused variable.

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

Impact

If the msg.sender passed to CharityRegistry is an EOA account or a contract that is not an instance of CharityRegistry it will always return the registry.isVerified(charity) check to false making the donate function inaccessible.

Tools Used

Manual review.

Recommendations

Set the registry with the correct instance of the CharityRegistry i.e _registry passed in the constructor.

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
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.