Security advisory: Initialize UUPS implementation contracts

Thank you @spalladino I understand that an implementation contract should be initialised. Meanwhile, does this specific line of code, constructor() initializer {}, need to be included in the Smart Contract even if the Contract is being initialised? I am not sure I fully understand the use of that specific line of code. Is calling the function function initialize() initializer public {...} sufficient? Finally, does version 0.8.0 protect against the vulnerability in question? Thank you again.

constructor() initializer {} is generally enough to fully initialize the contract. If you want to initialize it manually separately you will need to omit that line.

The latest versions of OpenZeppelin Contracts are not vulnerable to this vulnerability.

Thank you @frangio I have upgraded the OpenZeppelin Upgradeable Contracts to version 4.3.2. I am using the following in my Smart Contract, and do not have any Constructor in my Contract itself:

contract MyContract is Initializable, ERC721Upgradeable, OwnableUpgradeable, PausableUpgradeable, ReentrancyGuardUpgradeable, 
ERC721EnumerableUpgradeable, ERC721URIStorageUpgradeable {

    function initialize() initializer public {
        __ERC721_init("MyToken", "MYT");
        __ERC721Enumerable_init();
        __ERC721URIStorage_init();
        __Ownable_init();
        __Pausable_init();
        __ReentrancyGuard_init();
    }

...
}

However, when I include the line constructor() initializer {} I receive the following error, which appears to point to OpenZeppelin and Truffle modules:

../project:/contracts/MyContract.sol:15: Contract `MyContract` has a constructor
    Define an initializer instead
    https://zpl.in/upgrades/error-001
    at Object.assertUpgradeSafe (.../node_modules/@openzeppelin/upgrades-core/src/validate/query.ts:17:11)
    at Object.deployImpl (.../node_modules/@openzeppelin/truffle-upgrades/src/utils/deploy-impl.ts:33:3)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at deployProxy (.../node_modules/@openzeppelin/truffle-upgrades/src/deploy-proxy.ts:46:16)
    at module.exports (.../migrations/2_deploy_contract.js:9:3)
    at Migration._deploy (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/Migration.js:75:1)
    at Migration._load (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/Migration.js:56:1)
    at Migration.run (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/Migration.js:217:1)
    at Object.runMigrations (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:150:1)
    at Object.runFrom (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:110:1)
    at Object.runAll (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:114:1)
    at Object.run (/usr/lib/node_modules/truffle/build/webpack:/packages/migrate/index.js:79:1)
    at runMigrations (/usr/lib/node_modules/truffle/build/webpack:/packages/core/lib/commands/migrate.js:258:1)
    at Object.run (/usr/lib/node_modules/truffle/build/webpack:/packages/core/lib/commands/migrate.js:223:1)
    at Command.run (/usr/lib/node_modules/truffle/build/webpack:/packages/core/lib/command.js:183:1)
Truffle v5.4.13 (core: 5.4.13)
Node v14.18.1

If I do not include that line, the Contract deploys properly.

Thank you again. J

Hey @JF0001, can you try adding the following line immediately above the constructor?

/// @custom:oz-upgrades-unsafe-allow constructor

This will tell the upgrade plugins that you know what you're doing, and let you keep the constructor. The latest version of the wizard generates upgradeable contracts using this pattern.

2 Likes

Thank you @spalladino That works!

1 Like

Hello @spalladino, i have a question about add this. I had deployed a contract before see this post last week. My contract code didn't include this constructor() initializer method. I only had this

   function initialize() public initializer {
        __ERC20_init("SIMPLE", "SIMPLE");
        __ERC20Burnable_init();
        __Pausable_init();
        __AccessControl_init();
        __Ownable_init();
        __UUPSUpgradeable_init();

        _grantRole(DEFAULT_ADMIN_ROLE, msg.sender);
        _grantRole(PAUSER_ROLE, msg.sender);
        _grantRole(MINTER_ROLE, msg.sender);
        _grantRole(UPGRADER_ROLE, msg.sender);
    }

I am current using this version openzeppelin/contracts-upgradeable": "^4.4.1
If I would need to add this constructor, it is possible that I add it on upgrade? Does it work if adding this by upgrade? Or I need to deploy a new contract instead?

@xfss_foo You can send a transaction to your implementation contract to invoke the initialize() function to make sure no one else can do it.

1 Like

3 posts were split to a new topic: Is my existing implementation initializable?

Since this vulnerability only affects the UUPS inherited smart contracts why does the Transparent proxy code generated by the Openzepplin wizard have the _disableInitializers

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

It is considered good practice to disable initializers regardless of the upgrade pattern being used, as an additional measure. (We weren't explicitly recommending this before, but we do since this advisory was published.)

1 Like