The Standard

Summary

Changing the owner address in a single-step process allows the owner to transfer ownership to a non-existent or mistyped address and potentially locks all functions that use the onlyOwner modifier.

Vulnerability Details

The currently used 1-step process (changing the owner address using the setOwner() function) is risky, because it allows to transfer ownership to a non-existent or mistyped address. .

Impact

If by accident, the ownership is transferred to the wrong address (zero address mistyped address), key features of the contract become unusable. In our contract, the following functions would no longer be able to be called: removeCollateralNative(),removeCollateral(), removeAsset(), mint() and swap()

Tools Used

Manual Review

Recommendations

Instead, a 2-step process should be used to change the owner address. In the first step, the current owner should call a method to transfer the ownership. And, in the second step, the new owner needs to call a method to accept the ownership.

The easiest way to implement this is by using Ownable2Step.sol contract from OpenZeppelin, which provides the transferOwnership(address newOwner) function to initiate the transfer and the function acceptOwnership() that needs to be called by the pending owner in order to accept the ownership of the contract and to finalize the ownership transfer.

The contract can be fund at: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/access/Ownable2Step.sol