Sparkn

Summary

The potential for human error makes the single-step critical ownership transfer process risky, as mistakes could lead to the unintended locking of all functions utilizing the onlyOwner modifier.

Vulnerability Details

The Ownable.sol custom contract is inherited by proxyFactory.sol to incorporate ownable functionality. Presently, the implementation lacks safety because it involves a one-step process, which poses a risk due to potential human errors. Such errors can lead to irreversible consequences. For instance, there's a chance of mistakenly passing an incorrect address, which might lack a known associated private key.

Impact

Critical functions using the onlyOwner modifier will be locked , such as setContest() and deployProxyAndDistributeByOwner().

Tools Used

manual review

Recommendations

To ensure a more secure ownership change process and reduce potential risks, the following two-step approach can be implemented:

Step 1: Approval of Pending Ownership
Begin by designating a new address as the pendingOwner through a dedicated function. This step merely establishes the pending ownership without immediately effecting the change.

Step 2: Claiming Ownership Change
Once the pendingOwner address has been approved, the ownership change can be finalized. This involves the pendingOwner initiating a transaction to claim the ownership change. This step ensures that only after the correct address has been approved in step 1 can the ownership change be completed successfully in step 2. This way, the risk associated with errors is minimized, as incorrect addresses can be rectified during the initial approval step.