Submission Details
Severity:
When GivingThanks is deployed CharityRegistry is not set
Summary
'GivingThanks' contract constructor has a parameter for 'CharityRegistry' contract address. However, when 'GivingThanks' is deployed, the 'CharityRegistry' store variable is initialized with 'msg.sender' and will cause future donations to fail, as indicated by the failure of the tests 'GivingThanksTest::testDonate()' and 'GivingThanksTest::testFuzzDonate()'.
Vulnerability Details
It is not a vulnerability as such, it is a bad initialization in the deployment that affects tests
Impact
Deploying the 'Giving Thanks' contract without a 'Charity Registry' contract could cause problems with donations to registered and verified addresses until the registry is updated using the 'GivingThanks::updateRegistry' function. The tests 'GivingThanksTest::testDonate()', 'GivingThanksTest::testFuzzDonate()' and 'GivingThanksTest::testCannotDonateToUnverifiedCharity()' give a false positive/negative because of this incomplete initialization.
Tools Used
Foundry , manual code review
Recommendations
The CharityRegistry variable must be initialized with the address of the previously created CharityRegistry contract.
``` diff
constructor(address _registry) ERC721("DonationReceipt", "DRC") {
- registry = CharityRegistry(msg.sender);
+ registry = CharityRegistry(_registry);
owner = msg.sender;
tokenCounter = 0;
}
```
Updates
Lead Judging Commences
n0kto 12 months ago
Submission Judgement Published 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.
Rewards breakdown
nSLOC
67This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.