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

Msg.sender is set instead of the address of the charityRegistry contract

joseph_nwodoh

Summary

msg.sender is set as the registry in the constructor instead of the address of the CharityRegistry

Vulnerability Details

In the GivingThanks constructor, msg.sender was set as the registry instead of the address of the CharityRegistry contract. This affected the major functionality in the GivingThanks::donate() where the registry checks if a particular charity address is verified by the admin. the registry is set as msg.sender, therefore the GivingThanks::donate() function reverts.

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

Impact

The GivingThanks::Donate() function reverts, which means no eth can be sent by a donor to a verified charity.

Tools Used

foundry, manual analysis

Recommendations

change the CharityRegistry parameter in the GivingThanks constructor from msg.sender to the appropriate registry paramter.

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