OpenZeppelin / OpenZeppelin/openzeppelin-contracts
Improve security for Initializer.sol
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
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- 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