OpenZeppelin / OpenZeppelin/openzeppelin-contracts

Improve security for Initializer.sol

Open
#4,342 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Solidity
Stars
27.2k
Forks
12.4k
Avg merge
2d 19h
Merged PRs (30d)
33

Description

Motivation
Vulnerabilities, such as the Wormhole uninitialized proxy issue should not happen. Requiring a call to _disableInitializers() in a proxy's implementation contract is a confusing and error-prone step.

Details
Initializer.sol should disallow a call to an initializer function by default in the implementation contract itself for proxies and clones. This could achieved in the constructor:

abstract contract InitializableUpgradeable {
    // ...

    constructor() {
        _disableInitializers();
    }
}

Or by including an immutable reference to the implementation address:

abstract contract InitializableUpgradeable {
    // ...

    address private immutable self = address(this);
    
    modifier initializer() {
        require(address(this) != self, "InitializableUpgradeable: unable to initialize implementation");
        // ...
    }
}

For the sake of security, I would propose to not have both Initializer.sol and InitializerUpgradeable.sol co-exist. If for some reason the old behavior should be kept, however, I would recommend to be explicit in the naming of the modifiers. By this I mean naming the two modifiers to something verbose, like initializerUnsafe which has the current behavior and initializerUpgradeable which disallows calls in the implementation contract.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with Initializer.sol and InitializerUpgradeable.sol, then inspect the initializer modifier and the proposed constructor or implementation-address checks. Compare the behavior needed for proxies and clones, and determine whether the two contracts and modifier behaviors should coexist. Done means implementation contracts cannot be initialized directly without breaking supported proxy and clone usage.

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity
Domain
blockchain, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.