Description

Both the GivingThanks and CharityRegistry contracts fail to properly validate addresses that could potentially be the zero address (0x0). Specifically, the GivingThanks contract allows donations to be sent to an unvalidated charity address, which may be the zero address, resulting in the loss of funds. Similarly, the CharityRegistry contract does not check for the zero address when registering or verifying charity addresses, potentially allowing a zero address to be incorrectly handled as a valid charity.

Impact

GivingThanks Contract: Donors could unknowingly donate Ether to the zero address, which cannot accept funds. This results in the loss of donations since the Ether sent to the zero address is effectively "burnt."
CharityRegistry Contract: The registry may incorrectly allow the registration or verification of a zero address as a valid charity, causing logical issues in the system, including the possibility of sending donations to an invalid address.

Proof Of Concept

In the GivingThanks contract, the donor could invoke the ThanksGiving::donate function with the zero address, causing the donation to be sent to 0x0. The contract would mint an NFT for the donor, but no meaningful action would occur because the funds cannot be received by the zero address.

In the CharityRegistry contract, if a zero address is registered or verified as a charity, the contract may fail to properly identify valid charities, leading to invalid interactions with the GivingThanks contract or other components that rely on the registry.

Recommended mitigation

In the GivingThanks contract: Add a check in the GivingThanks::donate function to ensure that the provided charity address is not the zero address before attempting to send funds:

In the CharityRegistry contract: Ensure that the CharityRegistry::registerCharity and CharityRegistry::verifyCharity functions validate that the charity address is not 0x0 address before proceeding with any logic that registers or verifies the address: