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

`msg.sender` is used instead of _registry which cuases `isVerified()` to fail.

kobbyeugene

Summary

The constructor takes a registry parameter but ignores it and uses msg.sender instead. This means the CharityRegistry is being set to the wrong address, which would cause isVerified() checks to fail.

Vulnerability Details

The constructor of the GivingThanks contract takes a _registry parameter but instead of using it, it sets the registry variable to msg.sender. This means the CharityRegistry is being set to the wrong address, which would cause the isVerified() function to always return false. This affects the whole functionality of the GivingThanks contract, causing it to revert when a donor tries to donate.

Impact

The donate() function in the GivingThanks contract will always fail the require(registry.isVerified(charity), "Charity not verified") check, because the CharityRegistry is set to the wrong address.

Tools Used

Manual review

Recommendations

The constructor of the GivingThanks contract should be updated to correctly set the registry variable using the provided _registry parameter, as shown in the provided.

```diff
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 about 1 year 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.

Give us feedback!