The GivingThanks contract incorrectly assigns the deployer's address (msg.sender) to the registry state variable in the constructor, rather than using the address passed as the _registry argument. This results in the contract referencing the wrong CharityRegistry contract, which can cause it to malfunction and potentially fail in interacting with the correct registry.
In the GivingThanks.sol contract constructor:
The contract attempts to assign the msg.sender address to the registry state variable. However, the intention is to pass a specific CharityRegistry contract address during deployment via the _registry parameter, not use the deployer's address.
Problem:
The GivingThanks contract’s constructor is mistakenly using msg.sender (the address deploying the GivingThanks contract) instead of the _registry argument.
This results in the GivingThanks contract pointing to the deployer's address, not the actual CharityRegistry contract.
As a result, functions depending on this incorrect reference will fail to interact with the intended CharityRegistry.
Operational Impact: The contract will not interact with the intended CharityRegistry, leading to failure in executing charity-related functions.
Security Risk: Unauthorized contracts could be referenced if msg.sender is used inappropriately, leading to potential privilege escalation or malicious actions.
User Trust: Users may lose confidence in the system if the contract fails to operate as expected.
Manual Code Review: Identified the issue through examination of the constructor logic.
To resolve this issue, the constructor should correctly assign the _registry argument to the registry state variable, as follows:
This is a high-severity issue because it disrupts critical functionality by causing the contract to interact with the wrong registry address, leading to failures in essential operations. Furthermore, it introduces potential security risks by referencing an unintended address, which could be exploited for malicious purposes.
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.