GivingThanks

Root Cause and Impact

• Root Cause: The contracts assign address values to state variables and mappings without checking if the address is the zero address (address(0)).

• Impact: Assigning address(0) can lead to unexpected behavior, as it is often used as a sentinel value in Solidity to represent a null or uninitialized address. This could result in security risks, such as loss of access control or misdirected funds.

Vulnerability Details

• Assignments Without Zero Address Check:

  • In CharityRegistry:

    function registerCharity(address charity) public {

    registeredCharities[charity] = true;

    }

    • Issue: No check to ensure charity is not address(0).

    • Consequence: address(0) could be registered as a charity.

  • In GivingThanks:

    function updateRegistry(address _registry) public {

    registry = CharityRegistry(_registry);

    }

    • Issue: No validation of _registry.

    • Consequence: Setting registry to address(0) would break contract functionality.

Recommendations

• Add Zero Address Checks:

  • For registerCharity:

    function registerCharity(address charity) public onlyAdmin {

    require(charity != address(0), "Invalid charity address");

    registeredCharities[charity] = true;

    }

  • For updateRegistry:

    function updateRegistry(address _registry) public onlyOwner {

    require(_registry != address(0), "Registry address cannot be zero");

    registry = CharityRegistry(_registry);

    }

• General Best Practice:

  • Before assigning any address to a state variable or mapping, check that it is not address(0) to prevent unintended behaviors.