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

CharityRegistry's admin set wrong in the construction

dustinhuel2

Summary

The admin variable in CharityRegistry is set as msg.sender during contract deployment. If CharityRegistry is deployed by another contract (e.g., GivingThanks), msg.sender will be the address of the deploying contract rather than the actual deployer or intended admin. This can lead to a situation where the wrong entity has admin rights over the CharityRegistry.

Impact

This could result in the unintended loss of administrative control over the CharityRegistry, potentially preventing proper verification of charities or management of the registry.

Tools Used

manual

Recommendations

Modify the constructor to accept.

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 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.