GivingThanks

Summary

The contract contains a line, registry = CharityRegistry(msg.sender);, which may incorrectly assign the registry address based on the caller’s address rather than an explicitly provided address. This can lead to unintended or incorrect assignment of the CharityRegistry address, potentially resulting in critical functions relying on an incorrect registry. This issue can expose the contract to several vulnerabilities, such as unauthorized access or erroneous state changes.

Vulnerability Details

The vulnerability stems from assigning the registry variable to msg.sender, the address of the entity calling the function. If msg.sender is not the intended CharityRegistry contract but rather an external user or contract, this will assign the wrong address to registry. Consequently:

1. The contract may depend on the wrong registry for charity or donation information.

2. Functions dependent on registry could execute with unintended permissions or data sources.

The correct approach is to pass the CharityRegistry address explicitly to the contract, either through a constructor or an initialization function. This would ensure that the correct contract address is assigned to registry from the start.

Impact

• Unauthorized Access: If registry is set to an address controlled by a malicious actor, that entity could potentially influence or control critical functions within the contract.

• Data Corruption: Functions that rely on the CharityRegistry for donation, charity, or reward information might pull incorrect data, leading to incorrect state changes or fund allocations.

• Loss of Funds: Funds could be misallocated to unintended or malicious recipients if the CharityRegistry address is set incorrectly.

Tools Used

Tests

Recommendations

1. Use an Explicit Parameter for CharityRegistry Address:

   • Pass the intended CharityRegistry address as a parameter in the constructor or an initialization function to ensure the correct address is assigned. Replace msg.sender with an explicit _registry parameter, as shown below:

   ​

   `address public registry; constructor(address _registry) { registry = CharityRegistry(_registry); }`

2. Validate the Registry Address:

   • Include checks to confirm that the CharityRegistry address passed to the constructor or initializer is valid (non-zero address).

   • This prevents the assignment of uninitialized or zero addresses to registry, enhancing the contract’s security.

3. Access Control:

   • Ensure that functions interacting with registry have access control to prevent unauthorized calls or reassignments, particularly if the registry could influence sensitive functions.