Zaros Part 1

Summary

GlobalConfigurationBranch & UpgradeBranchimplement initialize() functions that reference the same initializer storage slot, meaning that if one of the initialziers is called the other one would fail.

Vulnerability Details

GlobalConfigurationBranch & UpgradeBranch both have initialize() functions and are both inherited in the PerpsEnginewhich is the RootProxy, meaning they share the same storage:

Both contracts inherit Initializable from OZ, which defines the following storage logic:

This means that both contracts reference the same InitializableStorage._initialized variable inside the Proxy. As a result if GlobalConfigurationBranch calls initialize()it will succeed, but calling UpgradeBranch.initialize()after that will revert, since the first initialization call would update InitializableStorage._initialized to true.

Tools Used

Manual Review

Recommendations

Since all contracts share the same Proxy (Diamond Proxy) instead each having it's separate proxy, storage is shared. This means that initialization function (with initializer modifier) can be only one.

Consider moving all the initialization logic either to UpgradeBranch or to GlobalConfigurationBranch. This way initialize() would be called only once and it will succeed.