# Optimism Smart Contracts Audit By [Hannah](https://paragraph.com/@ascensiveassets) · 2022-05-02 --- The [Optimism](https://optimism.io/) team is building the OVM, a fully-featured, EVM-compliant execution environment designed for Layer 2 systems. Starting on March 15th, 2021, we have audited Optimism’s code base during 7 weeks with 3 auditors. ### Scope The engagement involved auditing two different versions of the Solidity smart contracts: * The first audited commit was `18e128343731b9bde23812ce932e24d81440b6b7`. We worked with this commit for the first 4 weeks. * The second audited commit was `a935e276f5620b40802b52721e3474232e458f72`. We worked with this commit for the last 3 weeks. The Solidity files in scope were those in the `contracts/optimistic-ethereum/OVM/` and `contracts/optimistic-ethereum/libraries/`folders, except the `OVM_BondManager.sol`, `OVM_SafetyChecker.sol`, `ERC1820Registry.sol` and `OVM_DeployerWhitelist.sol` files. It should be noted that while the `Lib_RingBuffer.sol` file was originally included in the audit’s scope, after feedback during the audit the file has been deprecated and is expected to change in the short term – we have therefore not conducted a full assessment of the security of this particular library. Moreover, the specified commits contain production code that has been temporarily commented out. Our analysis assumes it will be restored. All other components of the stack (such as compilers, off-chain services, Layer 2 nodes, or bytecode checkers) were left completely out scope from the beginning of the audit. We have assumed any out-of-scope component behaves as intended and documented by the Optimism team. Furthermore, the Optimism team independently identified a number of issues in the code base that they shared with us during the audit. For completeness, these are included in an informational note titled **“\[N01\] Additional issues”**. ### Summary During the audit we were able to uncover issues of varied nature. The most important relate to critical security flaws in the fraud proof verification process, which is the fundamental mechanic ensuring security of the Layer 2 system. We also detected particularly interesting issues in cross-domain deposits and withdrawals of tokens, mishandling of transaction fees, mismatches in the treatment of sequenced and queued transactions, as well as potential abuses of the reward dynamics during fraud proof contribution. Additionally, while not strictly pertaining to security-related issues, this report includes a significant number of recommendations aimed at improving the overall quality of the system. In terms of specification and documentation, even though there have been notable advances, we still find that there is room for improvements. Gas accounting, upgradeability of core modules, genesis process, progressive decentralization roadmap, and interactions between Layer 1 contracts and off-chain services (such as the Sequencer), stand out as significant attention points in this regard. We acknowledge that the system is still under development, which can render documentation efforts pointless. Still, we expect that as the system matures and its fundamental mechanics are settled, the Optimism team will make efforts to produce a comprehensive baseline specification of their layer 2 solution that can serve as the bedrock upon which future versions of the system build. Regarding overall health and maturity, we found the code to be readable and well-structured, with enough separation of concerns and modularity to favor long-term maintenance and sustainability. Having audited an earlier version of the system in November 2020, this time we found a more robust code base with simpler and more straightforward implementations. The remarkable difference in number and severity of issues identified between audits is a reflection of how much the code base has matured in the last months. We expect the system to continue in this path as it further evolves, incorporating additional feedback from security-minded professionals, development partners and users alike. As a final remark, we must highlight that given the sandboxed environment allows for arbitrary code execution of Layer 2 transactions, the number of possible interactions cannot be audited to exhaustion. We therefore highly advise: * Following best practices of secure software development, thorough test-driven development, and mandatory peer-reviews. * Further promoting a public bug bounty program to engage independent security researchers from the community in uncovering further misbehaviors in the system as the code base evolves. * Continuing with beta testing phases until several projects have been onboarded to the Layer 2 system, and their dynamics have become battle-tested, in particular with respect to fraud proof verification in Layer 1. In this regard, we value the slow progressive decentralization approach taken by the Optimism team. ### System overview Documentation about the system main components can be found in [the official documentation](https://community.optimism.io/docs/protocol/protocol.html#system-overview) and [research articles](https://research.paradigm.xyz/optimism), as well as in our [original audit report](https://blog.openzeppelin.com/private-report-nov-9-2020-fs8a92/). On top of the core components audited in the early version of the system, in this audit we also included: * Bridge contracts, which essentially allow for message passing between Layer 1 and 2. * Predeploy contracts, which provide essential utilities in Layer 2 (such as a tokenized versioned of ETH, or decompression of sequenced calldata). ### Privileged roles * The owner of the `Lib_AddressManager` contract can arbitrarily add, delete and modify the addresses stored. It is therefore entitled to impersonate or change the logic of critical components of the system at will. * The Sequencer is a single, semi-trusted, off-chain service that is expect to process, order and append batches of transactions to the Canonical Transaction chain. * There are accounts that can propose state roots in the State Commitment chain. These accounts should have deposited a bond to be granted the role, and are expected to be penalized if they misbehave. * As discussed further in the report (see issue **“\[M03\] Initial state root cannot be challenged”**), the entity that provides the first state root to the State Commitment Chain can decide on the OVM’s genesis state. ### Update All serious issues have been fixed or acknowledged by the Optimism team. The code has been migrated to a new repository with a different commit history. Our review disregards all changes introduced by the migration, other pull requests, or unrelated changes within the reviewed pull requests. Below, our findings in order of importance. Critical severity ----------------- ### \[C01\] Possible state manipulation after execution of transactions with invalid gas limit The `run` function of the `OVM_ExecutionManager` contract is [executed during a fraud proof](https://github.com/ethereum-optimism/contracts/blob/18e128343731b9bde23812ce932e24d81440b6b7/contracts/optimistic-ethereum/OVM/verification/OVM_StateTransitioner.sol#L355) to move the associated State Transitioner from [pre-execution](https://github.com/ethereum-optimism/contracts/blob/18e128343731b9bde23812ce932e24d81440b6b7/contracts/optimistic-ethereum/OVM/verification/OVM_StateTransitioner.sol#L328) to [post-execution](https://github.com/ethereum-optimism/contracts/blob/18e128343731b9bde23812ce932e24d81440b6b7/contracts/optimistic-ethereum/OVM/verification/OVM_StateTransitioner.sol#L357) phase. Transactions with an invalid gas limit can still be run in a fraud proof, but they are expected to ultimately [result in a no-op](https://github.com/ethereum-optimism/contracts/blob/18e128343731b9bde23812ce932e24d81440b6b7/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol#L188). In other words, the pre- and post-state of a transaction with an invalid gas limit should be the same. Right before finishing execution of a regular transaction (that is, one that does not revert nor return early), the `run` function [resets to zero](https://github.com/ethereum-optimism/contracts/blob/18e128343731b9bde23812ce932e24d81440b6b7/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol#L211)the reference to the State Manager stored in the `ovmStateManager` state variable. However, this reference is not reset after finishing execution early [when the transaction’s gas limit is not valid](https://github.com/ethereum-optimism/contracts/blob/18e128343731b9bde23812ce932e24d81440b6b7/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol#L186-L189). As a result, the Execution Manager will still be able to execute calls to the associated State Manager, and in turn the State Manager will consider the caller Execution Manager as correctly authenticated. The described behavior allows for anyone to call sensitive functions of the Execution Manager contract right after the execution of `run`, which could result in arbitrary state modifications in the State Manager contract. As a consequence, it would not be possible to successfully finish the fraud proof in the State Transitioner contract. For example, a malicious actor could call the `ovmCREATEEOA` function of the `OVM_ExecutionManager` contract, creating [a new account in state](https://github.com/ethereum-optimism/contracts/blob/ec1afca9e15117608121377b15d66cb56084e52d/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol#L460), which would in turn [increment](https://github.com/ethereum-optimism/contracts/blob/ec1afca9e15117608121377b15d66cb56084e52d/contracts/optimistic-ethereum/OVM/execution/OVM_ExecutionManager.sol#L1216) the total number of uncommitted accounts in the State Manager. This would effectively prevent [completing the transition](https://github.com/ethereum-optimism/contracts/blob/ec1afca9e15117608121377b15d66cb56084e52d/contracts/optimistic-ethereum/OVM/verification/OVM_StateTransitioner.sol#L415-L418) in the `OVM_StateTransitioner` contract. Consider clearing the `ovmStateManager` state variable after execution of transactions with invalid gas limits. **_Update_**_: Fixed in_ [_pull request #366_](https://github.com/ethereum-optimism/contracts/pull/366) _of the archived_ `ethereum-optimism` repository. ### \[C02\] Partially shared keys with EXTENSION nodes mishandled Inside the `_walkNodePath` function of the `Lib_MerkleTrie` library, when an `EXTENSION` node is encountered [which shares some, but not all of its key](https://github.com/ben-chain/contracts-v2/blob/a935e276f5620b40802b52721e3474232e458f72/contracts/optimistic-ethereum/libraries/trie/Lib_MerkleTrie.sol#L322) with `keyRemainder`, the walk will move on to the node [which the](https://github.com/ben-chain/contracts-v2/blob/a935e276f5620b40802b52721e3474232e458f72/contracts/optimistic-ethereum/libraries/trie/Lib_MerkleTrie.sol#L325) `EXTENSION` node points to, but will only [increment the key index](https://github.com/ben-chain/contracts-v2/blob/a935e276f5620b40802b52721e3474232e458f72/contracts/optimistic-ethereum/libraries/trie/Lib_MerkleTrie.sol#L326) by the `sharedNibbleLength`. More specifically, when the `sharedNibbleLength` is not 0, it will be assumed that all nibbles are shared with the `EXTENSION` node. The walk will then move to the node which [the node links to](https://github.com/ben-chain/contracts-v2/blob/a935e276f5620b40802b52721e3474232e458f72/contracts/optimistic-ethereum/libraries/trie/Lib_MerkleTrie.sol#L325), while the key increment will be set to `sharedNibbleLength`. This will result in an incorrect `currentKeyIndex` on the next loop iteration. The `_walkNodePath` function will assume it has reached the node which the `EXTENSION` node points to, while the `currentKeyIndex` will correspond to a path in the middle of the `EXTENSION` node. This means that the Merkle Trie may incorrectly identify some elements and there may be multiple keys that map to the same element. During fraud proof execution the described flaw could cause the `OVM_StateTransitioner` contract to incorrectly update [storage](https://github.com/ben-chain/contracts-v2/blob/a935e276f5620b40802b52721e3474232e458f72/contracts/optimistic-ethereum/OVM/verification/OVM_StateTransitioner.sol#L430-L439) or [account](https://github.com/ben-chain/contracts-v2/blob/a935e276f5620b40802b52721e3474232e458f72/contracts/optimistic-ethereum/OVM/verification/OVM_StateTransitioner.sol#L391-L400) elements, which could lead to a security vulnerability where invalid fraud proofs would succeed due to incorrect updates in trie roots. Since the `_walkNodePath` function should identify the nearest sibling to the [key](https://github.com/ben-chain/contracts-v2/blob/a935e276f5620b40802b52721e3474232e458f72/contracts/optimistic-ethereum/libraries/trie/Lib_MerkleTrie.sol#L234) which is being “walked” to, consider modifying the `_walkNodePath` function so that it breaks out of the loop and returns when a non-fully matching `EXTENSION` key is found. **_Update_**_: Fixed in_ [_pull request #747_](https://github.com/ethereum-optimism/optimism/commit/ae1ac05d7032422a71caf25d16f6e548df5b8d7f)_._ --- *Originally published on [Hannah](https://paragraph.com/@ascensiveassets/optimism-smart-contracts-audit)*