Analysis of OZ TimelockController security vulnerability patch
… and why you must stay up to date!
On 26th of August OpenZeppelin tweeted about a security vulnerability in TimelockController smart contract and asked all projects to migrate.
The security advisory (link) mentions that the vulnerability “allows an actor with the executor role to escalate privileges”. It looked quite serious so I decided to check it out and here are the details of the patch and vulnerability. If you are developer, this article will show you why you should stay up to date with reported vulnerabilities and react to them asap.
When you look at the source code of the commit (available here: https://github.com/OpenZeppelin/openzeppelin-contracts/commit/cec4f2ef57495d8b1742d62846da212515d99dd5) you will see only one change: isOperationReady check added in _beforeCall function.
TimelockController logic
If you know how TimelockController contract works, you can skip this section.
Always the first thing to understand source code of a patch is to understand the logic of patched contract.
The TimelockController is used as the owner of controlled contracts (e.g. some configuration contract of DeFi protocol). Whenever a call is to be made on controlled contract (e.g. to update some configuration parameters) it is scheduled via TimelockController contract and delayed in time. It allows any users to react to change and either accept it or withdraw their assets if they do not agree with the change.
This process is achieved with two functions:
- schedule, which sets the timelock for a specific function call, and
- execute, which executes the scheduled function call after the delay has passed.
There is also a batch version (scheduleBatch) which is used to schedule a batch of function calls executed in one transaction.
Schedule functions simply set a timelock (which is a delayed timestamp) for a specific function call(s).
Execute function (its batch version to be exact) looks as following:
It simply calls the _beforeCall function, then _call function for each scheduled function call and _afterCall function at the end. Below are these functions (before the patch):
As you can see, the isOperationReady check is present in _afterCall function. So why is it important to put it also in _beforeCall function (that is what the patch does)?
Vulnerability analysis
The root cause of the vulnerability is that it does not follow the well-known Check-Effects-Interactions pattern.
Proposal execution firstly executes function calls (Interactions) and then checks whether these function calls should be executed, in _afterCall function (Checks). If you are able to bypass the check using function calls you can call any function on behalf of the timelock controller contract.
Let’s see an example of the privilege escalation attack!
Exploit
First, the attacker, who has the EXECUTOR role, must be able to schedule execution that will be accepted with no delay. Fortunately (for the attacker of course), there is a updateDelay function that allows to set the minimum delay to 0, thus allowing execution of a proposal in the same block.
Second, the TimelockController inherits from AccessControll and has the grantRole function which is quite powerful, because it allows to manage the contract. Also, the TimelockController is an admin of itself so the attacker is able to manage the controller.
Third, in order to bypass the isOperationReady check, the attacker must schedule a batch function calls that corresponds to the exploit. To avoid recursion, the attacker can use another contract and call TimelockConroller indirectly.
Now, to the code…
We first create constants for roles (l. 8–10), interface to generate function selectors later (l. 13–17) and list of proposers and executors. The attacker is one of the executors (l. 36) — that is the exploitation condition.
Next, we deploy a timelock controller with a predefined delay (l. 62–64) and check whether the attacker has ADMIN role (l. 67).
Now the attack is being started in one call of executeBatch function with 3 functions being called:
- updateDelay on timelock controller ceontract, setting delay to 0 and allowing to execute proposals in the same block they are submitted,
- grantRole on timelock controller to grant ADMIN role to the contract deployed by the attacker (we will come back to this contract in a second),
- attack on the contract deployed by the attacker, which has ADMIN role at this moment (explained below).
The last function call is used to bypass the isOperationReady check by scheduling a proposal of currently executed batch (calling scheduleBatch function). We cannot do it directly because scheduleBatch function takes the list of called functions in parameters, including a call of itself, therefore it would end up in infinite recursion in source code — if you cannot see it, try to implement is with direct call :)
The attack function from the attacker’s contract is the following:
The attacker’s contract will submit a proposal, therefore it needs a PROPOSER role. As it already has an ADMIN role, it can simply grant PROPOSAL role to itself (l. 20). Next, it executes the main part of the attack, granting the ADMIN role to the attacker’s EOA (l. 22).
Now, it must bypass the isOperationReady check. To do so, it simply calls scheduleBatch function submitting the proposal that is just being finished.
Conclusions
Smart contracts are quite tricky, especially when you are allows to call any function on any contract and do not follow Check-Effects-Interactions patter.
Developer must remember to follow the best practices and use publicly available resources, such as SCSVS (Smart Contract Security Verification Standard), to do security checks of their contracts.
They also must be up to date with reported vulnerabilities and update their vulnerable 3rd party contracts.
You think that your protocol needs security audit or consultation? Contact me on Twitter !
This example also shows that sometimes the patch is very easy but the exploit behind it is quite complicated and thus interesting. To call one function (grantRole) and achieve the vulnerability goal (privilege escalation) the attacker has to build whole path and non-trivial bypassed.
See you in next analysis!