CVE-2021-39167: Improper Privilege Management (CWE-269)
value: OpenZepplin is a library for smart contract development. In affected versions a vulnerability in TimelockController allowed an actor with the executor role to escalate privileges. Further details about the vulnerability will be disclosed at a later date. As a workaround revoke the executor role from accounts not strictly under the team's control. We recommend revoking all executors that are not also proposers. When applying this mitigation, ensure there is at least one proposer and executor remaining.
CWE description: The product does not properly assign, modify, track, or check privileges for an actor, creating an unintended sphere of control for that actor.
CVSS v3 base score: 9.8
Published: 2021-08-27T00:15Z
Repository: https://github.com/OpenZeppelin/openzeppelin-contracts
Language: Other
Files changed: CHANGELOG.md, contracts/governance/TimelockController.sol
Commit date: 2021-08-24 16:23:12 -0300
Commit message: Add additional isOperationReady check in TimelockController

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 23079afa..99972ed6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,11 @@
 # Changelog
 
+## 4.3.1
+
+ * `TimelockController`: Add additional isOperationReady check.
+
 ## 4.3.0 (2021-08-17)
 
  * `ERC2771Context`: use private variable from storage to store the forwarder address. Fixes issues where `_msgSender()` was not callable from constructors. ([#2754](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2754))
  * `EnumerableSet`: add `values()` functions that returns an array containing all values in a single call. ([#2768](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2768))
  * `Governor`: added a modular system of `Governor` contracts based on `GovernorAlpha` and `GovernorBravo`. ([#2672](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2672))
@@ -93,10 +97,14 @@ Make sure you're using git or another version control system to be able to recov
 
 ### How to upgrade from 4.0-beta.x
 
 Some further changes have been done between the different beta iterations. Transitions made during this period are configured in the `migrate-imports` script. Consequently, you can upgrade from any previous 4.0-beta.x version using the same script as described in the *How to upgrade from 3.x* section.
 
+## 3.4.2
+
+ * `TimelockController`: Add additional isOperationReady check.
+
 ## 3.4.1 (2021-03-03)
 
  * `ERC721`: made `_approve` an internal function (was private).
 
 ## 3.4.0 (2021-02-02)
diff --git a/contracts/governance/TimelockController.sol b/contracts/governance/TimelockController.sol
index 10aeb4ee..e52cb0d3 100644
--- a/contracts/governance/TimelockController.sol
+++ b/contracts/governance/TimelockController.sol
@@ -266,11 +266,11 @@ contract TimelockController is AccessControl {
         bytes calldata data,
         bytes32 predecessor,
         bytes32 salt
     ) public payable virtual onlyRoleOrOpenRole(EXECUTOR_ROLE) {
         bytes32 id = hashOperation(target, value, data, predecessor, salt);
-        _beforeCall(predecessor);
+        _beforeCall(id, predecessor);
         _call(id, 0, target, value, data);
         _afterCall(id);
     }
 
     /**
@@ -291,21 +291,22 @@ contract TimelockController is AccessControl {
     ) public payable virtual onlyRoleOrOpenRole(EXECUTOR_ROLE) {
         require(targets.length == values.length, "TimelockController: length mismatch");
         require(targets.length == datas.length, "TimelockController: length mismatch");
 
         bytes32 id = hashOperationBatch(targets, values, datas, predecessor, salt);
-        _beforeCall(predecessor);
+        _beforeCall(id, predecessor);
         for (uint256 i = 0; i < targets.length; ++i) {
             _call(id, i, targets[i], values[i], datas[i]);
         }
         _afterCall(id);
     }
 
     /**
      * @dev Checks before execution of an operation's calls.
      */
-    function _beforeCall(bytes32 predecessor) private view {
+    function _beforeCall(bytes32 id, bytes32 predecessor) private view {
+        require(isOperationReady(id), "TimelockController: operation is not ready");
         require(predecessor == bytes32(0) || isOperationDone(predecessor), "TimelockController: missing dependency");
     }
 
     /**
      * @dev Checks after execution of an operation's calls.

Vulnerable code:         _beforeCall(predecessor);
        _beforeCall(predecessor);
    function _beforeCall(bytes32 predecessor) private view {
Fixed code: ## 4.3.1

 * `TimelockController`: Add additional isOperationReady check.

## 3.4.2

 * `TimelockController`: Add additional isOperationReady check.

        _beforeCall(id, predecessor);
        _beforeCall(id, predecessor);
    function _beforeCall(bytes32 id, bytes32 predecessor) private view {
        require(isOperationReady(id), "TimelockController: operation is not ready");