Security advisory: Initialize UUPS implementation contracts

General Announcements
1

Update September 14th: We have published a security advisory and released Contracts v4.3.2 with a hotfix for this vulnerability. Still, it is now recommended you always initialize your implementation contracts as explained below.

Update September 16th: We have now published a post-mortem with a lot of detail at UUPSUpgradeable Vulnerability Post-mortem.

Due to a vulnerability in OpenZeppelin Contracts v4.1.0 through v4.3.1, all projects using the UUPS proxy pattern should initialize their implementation contracts.

To help mitigate this situation, we have already executed transactions to initialize over 150 implementation contracts from multiple projects we identified across Mainnet, Polygon, xDAI, Binance, and Avalanche. These transactions were sent from address 0x37E8d216c3f6c79eC695FBD0cB9842e62fB84370 via a batching contract at 0x310fAC62C976d8F6FDFA34332a56EA1a05493b5b on all networks.

As an example on how to execute the mitigation, given the following upgradeable ERC20 contract:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.2;

import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract MyToken is Initializable, ERC20Upgradeable, OwnableUpgradeable, UUPSUpgradeable {
    function initialize() initializer public {
        __ERC20_init("MyToken", "MTK");
        __Ownable_init();
        __UUPSUpgradeable_init();
    }

    function _authorizeUpgrade(address newImplementation)
        internal
        onlyOwner
        override
    {}
}

A transaction must be sent to the implementation contract (not the proxy) to invoke the initialize method. You can find out the address of the implementation contract given a proxy in Etherscan (if your contract source code is verified) on the “Read as Proxy” tab of the “Contract” section.

We will be releasing more information about the vulnerability along with a fix for the Contracts package next week, in order to give time to all projects to execute this mitigation.

6 Likes
2

Hey, guys. If your contract not verified in Etherscan or others, just use below code to get your implemention address.

const { getImplementationAddress } =  require('@openzeppelin/upgrades-core');
    
const implementationAddress = await getImplementationAddress(provider, proxyAddress);

console.log('contract implementation is:', implementationAddress);
7 Likes
3

Thanks for the warning!
Can you confirm it only applies to UUPS inherited SC and not for the Transparent Proxy one's?

4

Indeed, this only affects UUPS and not Transparent.

5

Hi @spalladino do the implementations need to be initialized with values that match the proxies? Or can they hold any value? I'm asking this because our project has multiple proxies sharing a single implementation contract.

6

@Ken_Chan They do not have to be values that match the proxies. In fact, it is better if they are gibberish values, so that the contract is left in an unusable state.

7

Hi @spalladino, @frangio thanks for working on this.
We have been one of the 150 projects benefited by the initialization that your team carried out last week.
The question is; Were the implementation contract roles assigned to a specific account or were they loaded with gibberish values?
Thanks again guys, we appreciate your effort !

8

Depends. If your initializer assigned roles to msg.sender, then the role was assigned to the batch contract, which we selfdestructed after the operation. If it accepted the address of each role as a parameter, then it was assigned to a dummy contract with the following code, required to pass the initializer checks we found on the multiple contracts.

contract Fake is IERC165 {
    function name() public pure returns (string memory) { return "Nil"; }
    function symbol() public pure returns (string memory) { return "NIL"; }
    function decimals() public pure returns (uint) { return 18; }

    function lp() public view returns (address) { return address(this); }
    function token0() public view returns (address) { return address(this); }
    function token1() public view returns (address) { return address(this); }
    function rewardsToken() public view returns (address) { return address(this); }
    function WETH() public view returns (address) { return address(this); }
    function factory() public view returns (address) { return address(this); }

    function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
        return interfaceId != 0xffffffff;
    }
}
9

Great! Thanks for the quick reply!

1 Like
10

Hey guys, thank you for this information!, If we were to stay at 4.3.1, would be ensuring the initialization of the Implementation contracts be enough?, The increased bytecode size of patch 4.3.2 is troublesome on many of my contracts! :dizzy_face:.

11

As long as you ensure that no one can call the upgradeTo or upgradeToAndCall methods directly on your implementation contracts, you're good!

1 Like
12

You made it crystal clear to me, thanks a lot Santiago!

13

We've just released a post-mortem with more details about this vulnerability here: UUPSUpgradeable Vulnerability Post-mortem.

14

Am I correct that if you're still in development, there is this easy fix to add this to your implementation?

    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() initializer {} // <== ADDING THIS

    function initialize(
        // ... arguments
    ) public initializer {
        __ERC20_init(_name, _symbol);
        __Ownable_init();

        // ... your initializer logic
    }

Basically, this added constructor ensures that on implementation contract, the real initializer cannot be called. Thus, as the owner of the contract is non-initialized, no one can do anything meaningful with the contract

On the other hand, it's impossible to call constructor in proxied versions, so the real initializer is the only initializer that can be used

1 Like
15

Hey @caffeinum! That's correct, we have updated our documentation to reflect that:

Initializing the Implementation Contract

Do not leave an implementation contract uninitialized. An uninitialized implementation contract can be taken over by an attacker, which may impact the proxy. You can either invoke the initializer manually, or you can include a constructor to automatically mark it as initialized when it is deployed:

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() initializer {}

Just make sure that flagging the implementation as initialized is enough for preventing any calls to upgradeTo on the implementation contract. Or make sure you're using the latest version of OpenZeppelin Contracts which includes the fix to prevent direct upgradeTo calls to the implementation.

1 Like
16

Hi, so just to be clear. If using the latest version of OpenZeppelin do we still need to add this fix?

/// @custom:oz-upgrades-unsafe-allow constructor
constructor() initializer {}

I see it's mentioned in the documentation and is still being generated by the Wizard.

17

Hey! While the latest version should protect against this vulnerability, we still recommend that you initialize implementation contracts for an extra layer of defense.

3 Likes
18

Thank you for the update. I just got a clue from this announcement.

19

They do not have to be values that match the proxies. In fact, it is better if they are gibberish values, so that the contract is left in an unusable state.

I am needing some fake wallet and contract addresses for this purpose. How to get one that isn't a security risk?

20

0x00..00 or 0x00..dead could be used.