Why having a constructor by-itself isn't upgrade-safe?

Support Upgrades
#1

After Aave upgradeability exploit I’ve been using a constructor in implementation contract to initialize it while it’s being deployed (in one transaction). The reasoning behind that was: the proxy will never call a constructor anyway, so leaving it I considered safe, but the logic itself will be initialized, so no such things as Aave had can happen to it.

Recently I was trying to automate my deployments with OZ-Upgrade plugins.

But it doesn’t like my constructor, saying this:

Error: Contract is not upgrade safe: Contract has a constructor

My constructor looks like this:

constructor() {
  initialize(address(1), address(1), "1", "1");
}

Is it really not safe? If yes - why?

Also I haven’t noticed any mentions about having to pre-initialize logic contracts in the docs - but leaving them unitialized with anyone could call initialize is a potential security issue. Or I’m missing something?

1 Like
#2

Welcome @vicnaum, and thank you for your question!

To be honest I think the error message we’ve chosen is inaccurate, because a constructor doesn’t necessarily render a contract unsafe. The thing is, having a constructor in your upgradeable contract is very likely an error, and we want to guide you to write an initializer function instead.

In cases like the one you showed, the constructor is used intentionally and in a safe way, but detecting these safe scenarios is hard for the plugins so they take the cautious route and consider it an error.

This only becomes a security issue when the logic contract contains specific functionality that could be abused. This always comes down to the ability to execute selfdestruct on the logic contract, which can also happen indirectly via delegatecall.

Our upgrades plugins rule out any such logic contracts, because delegatecall and selfdestruct are not allowed in them. This means it is not necessary to initialize them.

2 Likes