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

Wrong instantiation of GivingThanks:CharityRegistry with msg.sender

edmpulasky

Summary

Unused GivingThanks Constructor param _registry.

Vulnerability Details

During construction the CharityRegistry is being instantiated as msg.sender instead of _registry param.

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

Impact

Donations cannot proceed unless CharityRegistry gets updated.

Tools Used

  • Manual Review

CharityRegistry wrongly instantiated at construction. Hence, Donations cannot proceed.

  • Owner - Deployer: Deploys the protocol (_registry param unused)

function testImpededDonations() public {
// run setUp which deploys as follows
// new GivingThanks(address(registryContract));
vm.deal(donor, 10 ether);
vm.expectRevert();
vm.prank(donor);
charityContract.donate{ value: 1 ether }(charity);
}
​

Recommendations

Instantiate CharityRegistry with Constructor's param.

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