Skip to content

Commit

Permalink
Restrict upgrade to proxy context in UUPSUpgradeable
Browse files Browse the repository at this point in the history
Co-authored-by: Francisco Giordano <frangio.1@gmail.com>
  • Loading branch information
Amxx and frangio committed Sep 14, 2021
1 parent d02cc02 commit 6241995
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
* `AccessControl`: add internal `_grantRole(bytes32,address)` and `_revokeRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568))
* `AccessControl`: mark `_setupRole(bytes32,address)` as deprecated in favor of `_grantRole(bytes32,address)`. ([#2568](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/#2568))

## 4.3.2

* `UUPSUpgradeable`: Add modifiers to prevent `upgradeTo` and `upgradeToAndCall` being executed on any contract that is not the active ERC1967 proxy. This prevents these functions being called on implementation contracts or minimal ERC1167 clones, in particular.

## 4.3.1

* `TimelockController`: Add additional isOperationReady check.
Expand Down
22 changes: 19 additions & 3 deletions contracts/proxy/utils/UUPSUpgradeable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,32 @@ import "../ERC1967/ERC1967Upgrade.sol";
* _Available since v4.1._
*/
abstract contract UUPSUpgradeable is ERC1967Upgrade {
/// @custom:oz-upgrades-unsafe-allow state-variable-immutable state-variable-assignment
address private immutable __self = address(this);

/**
* @dev Check that the execution is being performed through a delegatecall call and that the execution context is
* a proxy contract with an implementation (as defined in ERC1967) pointing to self. This should only be the case
* for UUPS and transparent proxies that are using the current contract as their implementation. Execution of a
* function through ERC1167 minimal proxies (clones) would not normally pass this test, but is not guaranteed to
* fail.
*/
modifier onlyProxy() {
require(address(this) != __self, "Function must be called through delegatecall");
require(_getImplementation() == __self, "Function must be called through active proxy");
_;
}

/**
* @dev Upgrade the implementation of the proxy to `newImplementation`.
*
* Calls {_authorizeUpgrade}.
*
* Emits an {Upgraded} event.
*/
function upgradeTo(address newImplementation) external virtual {
function upgradeTo(address newImplementation) external virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallSecure(newImplementation, bytes(""), false);
_upgradeToAndCallSecure(newImplementation, new bytes(0), false);
}

/**
Expand All @@ -37,7 +53,7 @@ abstract contract UUPSUpgradeable is ERC1967Upgrade {
*
* Emits an {Upgraded} event.
*/
function upgradeToAndCall(address newImplementation, bytes memory data) external payable virtual {
function upgradeToAndCall(address newImplementation, bytes memory data) external payable virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallSecure(newImplementation, data, true);
}
Expand Down

0 comments on commit 6241995

Please sign in to comment.