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

GivingThanks contract is using the address of the deployer as a CharityRegistry

nurayko

Summary

The GivingThanks contract has field CharityRegistry public registry; . In the constructor function it's value is set to the address of the deployer of the contract which is wrong :
registry = CharityRegistry(msg.sender);

Impact

The GivingThanks contract will use the msg.sender address as a registry which is unexpected and wrong. Furthermore , the constructor accepts a parameter for registry address _registry which is not used and causes even more confusion. Since the deployer of the contract will not be a registry the contracts functionality that works with registry will not work.

Proof of Code

Add the following test to GivingThanks.t.sol:

function testRegistryMalfunction() public {
GivingThanks giving;
CharityRegistry registry;
vm.prank(admin);
registry = new CharityRegistry();
​
// Deploy the GivingThanks contract with the registry address
vm.prank(admin);
giving = new GivingThanks(address(registry));
​
assert(address(giving.registry()) == admin);
assert(address(giving.registry()) != address(registry));
​
}

Execute it by running forge test --mt testRegistryMalfunction in a shell.

Recommendations

Apply the following diff to use in the input parameter that is passed in the constructor and set the registry field.

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