GivingThanks

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

Critical Initializaton Vulnerability found in the `constructor`

Description: The GivingThanks::constructor() incorrectly uses msg.sender for the registry address instead of the provided _registry parameter. This means the registry address will be set to the deployer's address rather than the intended CharityRegistry contract.

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
registry = CharityRegistry(msg.sender); // Uses msg.sender instead of _registry
owner = msg.sender;
tokenCounter = 0;
}

Impact:

  • Contract will not be properly initialized

  • All donation attempts will fail

  • System completely unusable

Proof of Concept:

function testIncorrectInitialization() public {
address registryAddr = address(new CharityRegistry());
vm.prank(user1);
GivingThanks givingThanks = new GivingThanks(registryAddr);
// Registry address is set to user1 instead of registryAddr
assertEq(address(givingThanks.registry()), user1);
}

Recommended Mitigation: Check whether the _registry address is not same as the address of the user.

constructor(address _registry) ERC721("DonationReceipt", "DRC") {
require(_registry != address(0), "Invalid registry address");
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.