Is _disableInitializers() still needed in UUPS implementations?

Support Upgrades
1

In one of my UUPS proxy contracts deployed using version 4.9.2, I forgot to use _disableInitializers() in the constructor of the implementation contract. This oversight means anyone can initialize my implementation and become the owner. Although I have now initialized it myself, I am still curious if _disableInitializers() is necessary in versions > 4.9.

Here is the relevant code from UUPSUpgradeable.sol:

modifier onlyProxy() {
    require(address(this) != __self, "Function must be called through delegatecall");
    require(_getImplementation() == __self, "Function must be called through active proxy");
    _;
}

function upgradeToAndCall(address newImplementation, bytes memory data) public payable virtual onlyProxy {
    _authorizeUpgrade(newImplementation);
    _upgradeToAndCallUUPS(newImplementation, data, true);
}

Given that this code includes the onlyProxy modifier, which ensures upgradeToAndCall can only be called via delegatecall, the vulnerability discussed in the UUPSUpgradeable Vulnerability Post-mortem is not present here.

Is using _disableInitializers() just a recommendation, or is it strictly necessary due to some other vulnerability I might be missing?

Thanks

2

What you described should be sufficient to avoid the linked issue. _disableInitializers() is mainly an additional recommendation as a best practice. See Is _disableInitializers necessary? - #2 by ericglau

1 Like