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

GivingThanks.constructor(): not setting CharityRegistry contract's address correctly

k3ysk1lls

Summary

GivingThanks.constructor() assigns the CharityRegistry contract to msg.sender, not to the _registry sent as a parameter.

Vulnerability Details

When deploying the GivingThanks contract, the CharityRegistry public registry variable should be set to correctly point to the CharityRegistry contract. This is done by assigning the CharityRegistry address sent during deployment via the _registry parameter to the constructor. The constructor instead assigns the address msg.sender to the CharityRegistry contract.

Impact

Incorrect assignment of the CharityRegistry contract causes the entire GivingThanks to fail to function properly. It will revert every time someone wants to donate.

Tools Used

Manual review

Recommendations

Correct GivingThanks.constructor() as follows:

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