Table of Contents
Summary
- Type
- Oracles
- Timeline
- From 2025-07-29
- To 2025-07-31
- Languages
- Solidity
- Total Issues
- 15 (14 resolved)
- Critical Severity Issues
- 0 (0 resolved)
- High Severity Issues
- 0 (0 resolved)
- Medium Severity Issues
- 0 (0 resolved)
- Low Severity Issues
- 6 (6 resolved)
- Notes & Additional Information
- 7 (6 resolved)
- Client Reported Issues
- 2 (2 resolved)
Scope
OpenZeppelin audited the UMAprotocol/managed-oracle repository at commit fc03083. The files that were diff-audited were compared against their versions present in the UMAprotocol/protocol repository at commit 6a23be1.
In scope were the following files:
src
├── common
│ ├── implementation
│ │ ├── AddressWhitelist.sol (diff)
│ │ └── DisableableAddressWhitelist.sol
│ └── interfaces
│ └── DisableableAddressWhitelistInterface.sol
└── optimistic-oracle-v2
└── implementation
├── ManagedOptimisticOracleV2.sol
└── OptimisticOracleV2.sol (diff)
The final state of the audited codebase, including all implemented resolutions, is reflected in commit 5b33321.
System Overview
The UMA Optimistic Oracle protocol is designed for the efficient and rapid resolution of data requests on-chain. It operates on a "propose and dispute" model, whereby a proposed value is accepted as true after a set "liveness" period, provided no one challenges it. A proposer stakes a bond to submit a price, and if another party believes that the price is incorrect, they can also stake a bond to dispute it.
An undisputed proposal is settled optimistically, rewarding the proposer, while a disputed price is escalated to UMA's Data Verification Mechanism (DVM) for ultimate resolution, which then determines the winner and allocates the bonds and rewards accordingly. This core protocol is extended by the ManagedOptimisticOracleV2
, which introduces a layer of administrative control, allowing designated managers to customize parameters like bond sizes and liveness periods for specific requests.
New Contracts
The ManagedOptimisticOracleV2
contract implements a multi-tiered, role-based access-control system to govern the oracle's operational parameters. At the highest level, the DEFAULT_ADMIN_ROLE
holds the exclusive authority to upgrade the contract's implementation. Below this, the REGULAR_ADMIN
role is responsible for managing system-wide settings, which includes configuring the default proposer and requester whitelists, and setting constraints such as the maximum bond and minimum liveness period. The contract also introduces a REQUEST_MANAGER
role, which is managed by the REGULAR_ADMIN
. This role has the ability to apply custom configurations to individual price requests, allowing it to override default parameters by setting specific bond amounts, liveness periods, and proposer whitelists on a per-request basis.
The DisableableAddressWhitelist
contract extends the base AddressWhitelist
contract by introducing a mechanism to toggle its enforcement. When enforcement is disabled, all addresses are considered whitelisted, effectively bypassing the underlying whitelist checks.
Diff Changes
The AddressWhitelist
contract was updated to ensure compatibility with the latest OpenZeppelin version and to improve its extensibility for inheriting contracts. The OptimisticOracleV2
contract was refactored to support upgradeability using the UUPS proxy pattern, and several of its functions have been made virtual to allow for extension by child contracts like ManagedOptimisticOracleV2
. This refactoring also modernized the code by replacing the SafeMath
library with Solidity's native overflow protection.
Security Model and Trust Assumptions
As mentioned above, the ManagedOptimisticOracleV2
contract involves multiple roles, and the security model of this system relies on a layered approach to access control and a set of critical trust assumptions regarding its administrators and configurations. The entities in charge of these roles are expected to follow best practices for account security, including robust key management and multi-factor authentication where applicable. The integrity and proper functioning of the protocol are contingent upon these assumptions always holding true.
- It is assumed that entities holding the
DEFAULT_ADMIN_ROLE
,REGULAR_ADMIN
, andREQUEST_MANAGER
roles operate in the best interest of the protocol and its users. The owner of theAddressWhitelist
andDisableableAddressWhitelist
contracts is trusted to manage the whitelist entries and enforcement status appropriately. - The proper setup and ongoing maintenance of the
requesterWhitelist
,defaultProposerWhitelist
, and anycustomProposerWhitelists
are critical for controlling access and participation. - The
DisableableAddressWhitelist
mechanism'sisEnforced
state is assumed to be managed correctly and in accordance with the desired access policies. - The whitelists used in the
ManagedOptimisticOracleV2
contract are expected to match the implementation of theDisableableAddressWhitelist
contract. - Proposers and disputers must trust the requester's on-chain contract to honestly handle callbacks triggered by state transitions in the price request lifecycle. These callbacks can give the requester the ability to denial-of-service (DoS) transitions such as dispute or settlement.
Low Severity
Currencies Do Not Accept a Bond By Default
The ManagedOptimisticOracleV2
contract introduces per-currency maximum bond amounts that are stored in the maximumBonds
mapping. These limits can be set either during initialization by the contract initializer or later by an authorized admin. If no maximum bond is explicitly set for a given currency, the mapping returns its default value of zero. As a result, any non-zero bond passed to the contract will fail validation via the _validateBond()
function, effectively preventing the usage of that currency until configured.
If this behavior is intentional, consider clearly documenting it in both the code and user-facing documentation. Alternatively, instead of defaulting to rejection, consider changing the logic such that currencies with unset maximum bond values have no enforced limit.
Update: Resolved in pull request #22 at commit ab33c3e. The UMA team added two comments to clarify the default rejection of a currency when no bond range is configured.
The RequestManagerAdded
and RequestManagerRemoved
Events Can Be Wrongfully Emitted
The addRequestManager
and removeRequestManager
functions of the ManagedOptimisticOracleV2
contract are responsible for assigning and revoking the REQUEST_MANAGER
role and are implemented through the grantRole
and revokeRole
functions of AccessControlUpgradeable
.
These functions also emit the RequestManagerAdded
and RequestManagerRemoved
events unconditionally, regardless of whether the role was actually granted or revoked. This means that if addRequestManager
is called for an address that already has the REQUEST_MANAGER
role, or removeRequestManager
is called for an address that does not, the corresponding event will still be emitted, which could be misleading for off-chain services that rely on these events to track state changes. Furthermore, these events are redundant, considering the fact that the AccessControlUpgradeable
contract already emits the RoleGranted
and RoleRevoked
events.
Consider removing the RequestManagerAdded
and RequestManagerRemoved
events entirely.
Update: Resolved in pull request #23 at commit 46774d9.
Missing Interface Validation for Whitelist Contracts
The ManagedOptimisticOracleV2
contract includes several functions that accept external contract addresses expected to implement the DisableableAddressWhitelistInterface
, such as the setDefaultProposerWhitelist
, setRequesterWhitelist
, and requestManagerSetProposerWhitelist
functions. However, these functions do not verify whether the provided addresses actually implement the expected interface, which can result in runtime errors or incorrect behavior if an invalid contract is passed.
Consider supporting the ERC-165 standard in both the AddressWhitelist
and DisableableAddressWhitelist
contracts to facilitate safe and standardized interface detection. In addition, consider enforcing interface compliance through runtime checks to ensure that the provided whitelist contracts implement the DisableableAddressWhitelistInterface
.
Update: Resolved in pull request #17 at commit 7bb517c. The DisableableAddressWhitelist
contract was replaced with DisabledAddressWhitelist
, thereby changing the implementation from an owner-controlled toggle for the enforcement of the list to a hard-coded implementation that can be set in the ManagedOptimisticOracleV2
contract. Both the DisabledAddressWhitelist
and AddressWhitelist
contract now extend ERC165
while ManagedOptimisticOracleV2
performs runtime checks whenever a whitelist is set.
Missing Test Suite
The in-scope contracts, including OptimisticOracleV2
and ManagedOptimisticOracleV2
, implement a sophisticated system for managing optimistic price requests. This system involves complex state transitions, financial stakes in the form of bonds, and a multi-layered access-control model. As such, the security and reliability of dependent protocols are critically reliant on the correctness of this logic.
Throughout the repository, there is an absence of a dedicated test suite for the contracts in scope. Without automated tests, there is no formal, repeatable process to verify that the contracts function as intended. Verifying the correctness of the intricate state machine, access-control logic, and financial calculations is left to manual review, which is error-prone and time-consuming. Furthermore, the lack of a regression suite means that future changes to the codebase could inadvertently introduce critical vulnerabilities without being detected.
Consider implementing a comprehensive test suite that covers the functionality of the scoped contracts. This suite should include unit tests for individual functions, integration tests to validate the interactions between contracts, and fork tests to simulate behavior in a realistic on-chain environment. The tests should validate expected outcomes, proper handling of edge cases, and adherence to the access control model. Establishing robust test coverage would significantly improve the maintainability and security posture of the system.
Update: Resolved in pull request #25 at commit 31b47fd. Tests for the ManagedOptimisticOracleV2
contract were added.
Problematic Whitelist Implementation
The AddressWhitelist
contract provides functionality for managing lists of approved addresses. It uses a mapping to store the status of an address and a separate, dynamic array, whitelistIndices
, to store every address ever added. The getWhitelist
function iterates through this array to return a list of currently active members.
The current implementation of the whitelist has several drawbacks:
- The
removeFromWhitelist
function only changes the status of an address toOut
but does not remove it from thewhitelistIndices
array. This "soft delete" causes the array to grow indefinitely, leading to an increasing gas cost for enumeration. - The
getWhitelist
function iterates through this ever-growing array twice, making it highly inefficient and creating a potential out-of-gas condition if the list becomes too large, as commented. - The usage of the three statuses (
None
,Out
, andIn
) is not only redundant compared to only two (Out
andIn
), it also introduces an inconsistent state. First, if an address is removed from the whitelist despite having never been added, its status is set toOut
. Second, if the address is then added to the whitelist, the address is not added towhitelistIndices
but still marked asIn
with an event emitted, although the address will not be included ingetWhitelist
. - Since the project already utilizes the OpenZeppelin Contracts library, this custom implementation also introduces redundant code, as a more robust and gas-efficient alternative is already available in the
EnumerableSet
library.
Consider refactoring the AddressWhitelist
contract to use the EnumerableSet.AddressSet
library provided by OpenZeppelin. Be aware that this adoption would shift the storage layout. As such, upgradeable contracts depending on AddressWhitelist
should be handled with care.
Update: Resolved in pull request #24 at commit 81bd067.
Minimum Liveness Can Be Set Beyond Valid Bounds
The setMinimumLiveness
function of the ManagedOptimisticOracleV2
contract allows the admin to set a new global minimum liveness without validating that the value falls within the bounds enforced by the underlying OptimisticOracleV2
. If minimumLiveness
is set above the maximum value accepted by OptimisticOracleV2,
subsequent attempts of a requester or request manager to set custom liveness values will fail, resulting in a DoS for that functionality.
While this scenario is unlikely to materialize in practice, as it requires a misconfigured call from a privileged admin, the risk can be fully mitigated by adding validation. Thus, consider updating the _setMinimumLiveness
function to enforce that the new value respects the bounds accepted by OptimisticOracleV2
, preventing any accidental misconfiguration.
Update: Resolved in pull request #7 at commit bd0fe60.
Notes & Additional Information
Use of Storage Gaps for Upgradeability
The OptimisticOracleV2.sol
contract is designed to be upgradeable and currently uses the storage gap pattern (uint256[998] private __gap;
) to reserve space for future state variables. This is a widely used technique to prevent storage collisions in child contracts when the parent contract is upgraded with new variables.
However, while functional, the storage gap pattern is a manual and somewhat opaque method for managing upgradeability. It does not provide a clear, structured approach for how storage should be organized, especially in contracts with complex inheritance. This can make reasoning about the overall storage layout difficult for developers and auditors.
Consider adopting the namespaced storage pattern defined in EIP-7201. This standard provides a robust and explicit convention for managing storage layouts in upgradeable contracts. By grouping related state variables into structs and assigning them a unique, deterministic storage location based on a namespace ID, EIP-7201 makes the storage layout modular and easier to understand. Adopting this standard would improve the long-term maintainability and developer ergonomics of the contract's upgradeability strategy.
Update: Acknowledged, not resolved. The team stated:
This is a pretty involved change and will break storage assumptions of the contracts already deployed (since last week) which are still using
__gap
instead. Thanks for bringing this up though, we will consider using this pattern in future contracts.
Inconsistent Use of Re-entrancy Guard on view
Functions
Throughout the OptimisticOracleV2
contract, external view
functions such as getRequest
and getState
are protected with the nonReentrantView
modifier. This modifier prevents these functions from being called back into during the execution of a state-changing function, ensuring that they cannot read from a temporarily inconsistent state.
In the ManagedOptimisticOracleV2
contract, new external view
functions have been introduced, namely getCustomProposerWhitelist
and getProposerWhitelistWithEnforcementStatus
. However, these functions, do not use the nonReentrantView
modifier. This creates an inconsistency with the established security pattern of the parent contract. While no direct exploit was identified from this omission due to robust state checks elsewhere, it represents a deviation from parent contract.
Consider adding the nonReentrantView
modifier to all new external
view
functions in ManagedOptimisticOracleV2
. This would enforce a consistent security posture across the entire contract system, improve clarity for developers and auditors, and harden the contract against potential re-entrancy vectors that could arise from future integrations.
Update: Resolved in pull request #28 at commit 77b6f82.
Code Improvement Opportunities
Throughout the codebase, multiple opportunities for code improvement were identified:
- To save gas at deployment and reduce the code size, consider combining the
_getManagedRequestId
andgetManagedRequestId
functions into a single,public
version. - The
ManagedOptimisticOracleV2Events
andManagedOptimisticOracleV2
contracts are redundantly split and do not improve legibility as they exist within the same file. Consider moving theManagedOptimisticOracleV2Events
contract into a separate file, or simply removing it and adding the events directly in theManagedOptimisticOracleV2
contract. - There are several casts to
address
that are redundant, such as the conversion ofmsg.sender
in therequestPrice
function, or the conversions ofrequester
in theproposePriceFor
,disputePriceFor
, and_settle
functions. Consider removing these unnecessary casts. - The codebase does not make use of named mapping parameters, neither in the
AddressWhitelist
contract nor in theManagedOptimisticOracleV2
contract. Consider enhancing mapping documentation by using named parameters. - The
newImplementation
name of theaddress
parameter in the_authorizeUpgrade
function is unused, consider removing it. - The
isSet
boolean flag in theCustomBond
andCustomLiveness
structs is unnecessary because its state can be inferred from the value ofamount
orliveness
, respectively. A zero value for either of these fields is not a sensible setting, so it can be used to signify that no custom value has been set, while any non-zero value indicates a custom setting is active. Removing this redundant boolean simplifies the logic and saves an extra storage slot for each struct, reducing the overall gas cost.
Consider addressing the identified instances to improve the quality and maintainability of the codebase.
Update: Resolved in pull request #27 at commit dc580dd.
Naming Suggestions
Throughout the codebase, multiple opportunities for naming improvements were identified:
- The term "whitelist" is used to describe a list of addresses that are permitted to perform certain actions. While historically common, the terms "whitelist" and "blacklist" can carry implicit connotations. In recent years, the technology industry has been moving towards more neutral and descriptive language that avoids potentially loaded terms. Consider replacing the term "whitelist" with "allowlist" throughout the codebase.
- To maintain consistency, the
REGULAR_ADMIN
andREQUEST_MANAGER
roles could be renamed toREGULAR_ADMIN_ROLE
andREQUEST_MANAGER_ROLE
. - The
liveness
field of theInitializeParams
struct could be renamed todefaultLiveness
.
To improve the legibility of the codebase, consider addressing the suggestions made above.
Update: Resolved in pull request #16 at commit acba673 and pull request #5 at commit f747326. The team stated:
We'd like to stick to whitelist naming for legacy reasons. Many systems use the whitelist naming. We will consider renaming in the future.
Use Custom Errors
Since Solidity version 0.8.4, custom errors provide a cleaner and more cost-efficient way to explain to users why an operation failed.
The OptimisticOracleV2
and ManagedOptimisticOracleV2
contracts use require
and revert
messages.
For conciseness and gas savings, consider replacing require
and revert
messages with custom errors.
Update: Resolved in pull request #8 at commit 7d085a8.
Missing Security Contact
Providing a specific security contact (such as an email address or an ENS name) within a smart contract significantly simplifies the process for individuals to communicate if they identify a vulnerability in the code. This practice is quite beneficial as it permits the code owners to dictate the communication channel for vulnerability disclosure, eliminating the risk of miscommunication or failure to report due to a lack of knowledge on how to do so. In addition, if the contract incorporates third-party libraries and a bug surfaces in those, it becomes easier for their maintainers to contact the appropriate person about the problem and provide mitigation instructions.
The OptimisticOracleV2
, ManagedOptimisticOracleV2
, AddressWhitelist
, and DisableableAddressWhitelist
contracts do not have a security contact.
Consider adding a NatSpec comment containing a security contact above each contract definition. Using the @custom:security-contact
convention is recommended as it has been adopted by the OpenZeppelin Wizard and the ethereum-lists.
Update: Resolved in pull request #10 at commit 210c517.
Function Visibility Overly Permissive
Throughout the codebase, multiple instances of functions with unnecessarily permissive visibility were identified:
In ManagedOptimisticOracleV2
:
- The
_setMaximumBond
function withinternal
visibility could be limited toprivate
. - The
_setMinimumLiveness
function withinternal
visibility could be limited toprivate
. - The
_setDefaultProposerWhitelist
function withinternal
visibility could be limited toprivate
. - The
_setRequesterWhitelist
function withinternal
visibility could be limited toprivate
. - The
_getManagedRequestId
function withinternal
visibility could be limited toprivate
. - The
_validateBond
function withinternal
visibility could be limited toprivate
. - The
_getEffectiveProposerWhitelist
function withinternal
visibility could be limited toprivate
.
In OptimisticOracleV2
:
- The
_getId
function withinternal
visibility could be limited toprivate
. - The
_getState
function withinternal
visibility could be limited toprivate
. - The
_getOracle
function withinternal
visibility could be limited toprivate
. - The
_getStore
function withinternal
visibility could be limited toprivate
. - The
_getIdentifierWhitelist
function withinternal
visibility could be limited toprivate
. - The
_getTimestampForDvmRequest
function withinternal
visibility could be limited toprivate
. - The
_stampAncillaryData
function withinternal
visibility could be limited toprivate
.
To better convey the intended use of functions, consider changing the functions' visibility to be only as permissive as required. Otherwise, consider documenting that the contracts are expected to be extended by third party developers.
Update: Resolved in pull request #26 at commit ce47fb4.
Client Reported
DisableableAddressWhitelist
Is Not Enforced By Default
The DisableableAddressWhitelist
contract extends the AddressWhitelist
contract and introduces the isEnforced
flag to control whether the whitelist logic is active or not. By default, this flag is not explicitly set during construction and therefore defaults to false
, meaning the whitelist is not enforced, and any address will pass the check. This creates a possibility for the owner to mistakenly assume that the whitelist is active without explicitly enabling it.
Consider modifying the DisableableAddressWhitelist
contract to set the isEnforced
flag to true
by default, ensuring that whitelist behavior is enforced unless explicitly disabled.
Update: Resolved in pull request #6 at commit 2a816e1. The UMA team stated:
Not relevant anymore, as we moved from
DisableableAddressWhitelist
to two separate contracts:DisabledAddressWhitelist
+AddressWhiteList
, which have clear behaviours consistent with their names
Misleading or Incomplete Documentation
Throughout the codebase, multiple instances of misleading or incomplete documentation were identified:
- The
@return
NatSpec of thegetCustomProposerWhitelist
function should be changed to@return DisableableAddressWhitelistInterface
, as per the implementation. - The
@dev
NatSpec of thegetManagedRequestId
function is incorrect.
Consider addressing the above instances to improve the clarity of the codebase.
Update: Resolved in pull request #9 at commit b8a7ea2. The first suggestion does no longer apply with the removal of the DisableableAddressWhitelistInterface
.
Conclusion
This report concludes a three-day audit of the managed-oracle protocol. Our analysis indicates that the protocol's design and implementation are fundamentally sound. We have identified several areas where code improvements can be made, and these have been detailed as findings in this report. These suggestions aim to enhance the robustness and efficiency of the existing codebase.
The codebase is well-commented and thoroughly documented, which greatly facilitated the review process. We would also like to commend the UMA team for their proactive approach in conducting their own internal audit, which had already identified several of the same points that we have raised. It was a pleasure to collaborate with them, and we appreciate their commitment to security and code quality.