GivingThanks

Summary : there are a couple of vulnerabilities i spotted.

1. lack of access control on updateRegistry function.

2. Improper registry assignment in the constructor.

Vulnerability Details :

1. In the constructor, owner is assigned as msg.sender, which overlaps with Ownable’s _owner variable. Since Ownable provides the onlyOwner modifier and other owner-based functionality, you can remove the owner variable from this contract to avoid confusion and rely on Ownable for ownership checks.

2. In the constructor, registry is set to CharityRegistry(msg.sender), which casts the deployer’s address as a CharityRegistry contract without verification. This is potentially dangerous, as msg.sender might not be the actual CharityRegistry address. Consider passing _registry as a constructor parameter directly to initialize the registry variable correctly

Impact :
It can lead to an unauthorized access by someone who is not the owner of this contract.

Tools Used :
chat GPT and the AI feature on Remix

Recommendations :
added a check to make sure that only the owner of trhe contract can make sensitive update on the contract.