The Standard

Summary

Vulnerability Details

The initialization function in SmartVaultManagerV5.sol can be called multiple times from the implementation contract.

Typically, in upgradeable contracts, an initialization function is safeguarded by a modifier to prevent unauthorized calls and ensure that it can only be executed once.

Impact

This vulnerability implies that a compromised implementation has the potential to reinitialize the aforementioned contract, seizing ownership and facilitating privilege escalation to drain users' funds.

Despite using the OpenZeppelin/Initializable.sol and making the contract SmartVaultManagerV5 Initializable.

That provides an Initializable base contract that has an initializer modifier that takes care of preventing a contract from being initialized multiple times.

Another difference between a constructor and a regular function is that Solidity takes care of automatically invoking the constructors of all ancestors of a contract.

When writing an initializer, you need to take special care to manually call the initializers of all parent contracts.

Note that the initializer modifier can only be called once even when using inheritance, so parent contracts should use the onlyInitializing modifier:

For further insights into potential risks and considerations when using initialize function, please consult the following resource:
https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#initializers

Tools Used

Manual Review

Recommendations

I reccomend first to protect the initialize function from being reinitiated with:

And, also add onlyInitializing as stated above: