The Standard

Summary

Different ownership implementations were found between three of the four contracts in scope:

• SmartVaultManagerV5 is using OwnableUpgradeable from OpenZeppelin.

• LiquidationPoolManager is using Ownable from OpenZeppelin.

• SmartVaultV3 is using a custom ownership logic.

However, none of them required the new proposed owner to accept the role. The lack of a 2-step process can lead to the loss of the ownership if a wrong address is used when changing the current owner.

Similar finding from other contests:

• https://solodit.xyz/issues/lack-of-two-step-ownership-pattern-halborn-floin-floin-s-pdf

• https://solodit.xyz/issues/l-04-use-code4rena-redacted-cartel-redacted-cartel-contest-git

• https://solodit.xyz/issues/l-01-use-code4rena-escher-escher-contest-git

• https://solodit.xyz/issues/single-step-ownership-change-is-risky-spearbit-none-nouns-dao-pdf

Vulnerability Details

All three ownership models explained above and pointed in the "Relevant GitHub Links" section above lack the ability to accept the ownership by the newly proposed owner.

Impact

If the new address is inactive or not willing to act in the role, there is no way to restore access to that role. Therefore, the owner role can be lost.

Tools Used

Manual analysis.

Recommendations

• For SmartVaultManagerV5 consider using Ownable2StepUpgradeable instead of OwnableUpgradeable.

• For LiquidationPoolManager consider using Ownable2Step instead of Ownable.

• For SmartVaultV3 consider implementing a proper 2-step approach for ownership transfers or using Ownable2Step by OpenZeppelin.