GivingThanks

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

Incorrect initialization of CharityRegistry address in GivingThanks Constructor

GivingThanks Constructor vulnerability

Summary

This report details a constructor vulnerability identified in the GivingThanks contract's constructor.

Vulnerability Details

In GivingThanks::constructor: msg.sender can not be both CharityRegistry and the caller, as CharityRegistry is not a utility contract, it does not implement the functionality of calling other contracts, deploying them. In other words, registry should be set to CharityRegistry(_registry) according to the README.

Impact

This vulnerability could lead to the GivingThanks contract being initialized with an incorrect CharityRegistry address, potentially causing donations to be sent to unintended recipients.

Tools Used

Formal Verification(None)

Recommendations

Change CharityRegistry(msg.sender) to CharityRegistry(_registry) in the constructor.

@@ -15,7 +15,7 @@
address public owner;
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.