GivingThanks

[L-1] One Step Ownership transfer can Lead to indefinite loss of the admin of the contract

Relevant Github Link: https://github.com/Cyfrin/2024-11-giving-thanks/blob/304812abfc16df934249ecd4cd8dea38568a625d/src/CharityRegistry.sol#L27

Description: The changeAdmin function in CharityRegistry allows the admin to transfer their ownership to a new address in a single step. However, if the newAdmin address provided is the zero address (0x000...000) or an unintended address (e.g., a typo or an incorrect entry), this function will update the admin to an unusable or non-existent account. This can lead to permanent loss of administrative control over the contract, with no way to revert or reassign the admin role. As a result, crucial functions that require admin privileges may become unusable, impacting the security and functionality of the contract.

Impact:

• Permanent loss of admin of the contract leading to:

  1. Inability to perform essential admin-only functions.

  2. Potential disruption of operations that depend on the admin role, affecting users of the protocol.

Recommended Mitigation:

1. A check if the newAdmin is a non-zero address.

2. A two step admin transfer can be done where the owner initiates the changeAdmin function and the newAdmin accepts the admin privilege. Check out: Opezeppelin Ownable2Step contract