The Standard

Summary

Single-step process for critical ownership transfer is risky due to possible human error which could result in locking all the functions that use the onlyOwner modifier in SmartVaultV3.sol

Vulnerability Details

In SmartVaultV3.sol, setOwner() function is used to change the owner of contract and this function can be only be called by onlyVaultManager.

The above implementation used in contract is not safe as the process is 1-step which is risky due to a possible human error and such an error is unrecoverable.

For example, an incorrect address, for which the private key is not known, could be passed accidentally.

In SmartVaultV3.sol functions using onlyOwner modifier like removeCollateralNative(), removeCollateral(), removeAsset(), mint() and swap() will be locked and can not be used if the owner address is set incorrectly and in worst case the whole SmartVaultV3 contract will be of no use if such critical functions can not be accessed by real owner. The users funds and the setter functions are at direct risk, Therefore the issue is identified as Medium severity.

Impact

The above discussed critical functions using the onlyOwner modifier in SmartVaultV3.sol will be locked and could not used if the issue happens.

Tools Used

Manual review

Recommendations

Implement the change of ownership in 2 steps:

1)Approve a new address as a pendingOwner

2)A transaction from the pendingOwner address claims the pending ownership change.

This mitigates the risk because if an incorrect address is used in step (1) then it can be fixed by re-approving the correct address. Only after a correct address is used in step (1) can step (2) happen and complete the ownership change.