GivingThanks

Summary

The constructor of the GivingThanks.sol contract initializes the registry variable, intended to reference an external CharityRegistry contract, to msg.sender instead of the _registry parameter. This creates a critical flaw in which GivingThanks.sol contract cannot access the actual CharityRegistry instance for operations such as verifying donations. Instead, the contract mistakenly sets the registry to the deployer of the GivingThanks contract, leading to erroneous interactions.

Vulnerability Details

Issue

• In the above constructor of GivingThanks.sol, the line registry = CharityRegistry(msg.sender); assigns msg.sender (the address deploying GivingThanks) to the registry variable rather than the intended _registry parameter, which is supposed to be the address of a valid CharityRegistry contract instance.

• The GivingThanks contract will treat the deployer's address as the CharityRegistry instance, causing functions such as donate to fail, as the deployer’s address cannot validate charity addresses or provide any registry functionality.

• It breaks the purpose of the GivingThanks contract, which relies on CharityRegistry contract to ensure only registered and verified charities receive donations.

Impact

• Donations will fail because the contract cannot verify charity addresses without a proper CharityRegistry instance, leading to failed transactions in the donate function.

Tools Used

• Manual Code Review

Recommendations

• Update the constructor to use the _registry parameter:

  constructor(address _registry) ERC721("DonationReceipt", "DRC") {

  registry = CharityRegistry(_registry);

  owner = msg.sender;

  tokenCounter = 0;

  }