Platform: Code4rena
Start Date: 18/10/2023
Pot Size: $36,500 USDC
Total HM: 17
Participants: 77
Period: 7 days
Judge: MiloTruck
Total Solo HM: 5
Id: 297
League: ETH
Rank: 12/77
Findings: 1
Award: $458.63
๐ Selected for report: 1
๐ Solo Findings: 0
๐ Selected for report: hunter_w3b
Also found by: 0xbrett8571, 0xweb3boy, JCK, Myd, SAAJ, ZanyBonzy, clara, fouzantanveer, jauvany, wei3erHase
458.6348 USDC - $458.63
Open Dollar is a DeFi lending protocol designed for liquidity and stability. Users can lock Liquid Staking Tokens (LSTs) and assets into Collateralized Debt Positions (CDPs) through NFT Vaults, allowing them to borrow the $OD stablecoin. Unlike other stablecoins, $OD adjusts its redemption rate based on market volatility, enhancing flexibility and minimizing governance. Open Dollar aims to reduce LST volatility, boost token liquidity, and offers a unique tradable vaults system. It's a floating $1.00 pegged stablecoin backed by LSTs, primarily for Arbitrum, and employs the GEB framework for CDPs. This protocol enables borrowing against staked assets, earning staking rewards, and liquidity through Non-Fungible Vaults (NFVs).
The key contracts of Open Doller protocol for this Audit are:
These contracts are central to the functionality, security, and governance of the Open Dollar protocol. Focusing on them first will provide a solid foundation for understanding the protocol's operation and how it manages user assets and stability.
ODGovernor.sol: ODGovernor contract manages the governance of the protocol, enabling modifications to key parameters. Understanding how governance functions is crucial for ensuring the protocol's stability and adaptability.
AccountingEngine.sol: The AccountingEngine contract handles the surplus, debt, and auctions. It's essential to comprehend how the protocol manages these aspects as they directly impact the stability and liquidity of the platform.
Vault721.sol: This contract serves as the Proxy Registry and Proxy Factory and is responsible for managing safe ownership, transfers, and approvals. Understanding how safes are managed is vital to ensure the security of users' assets.
ODProxy.sol: ODProxy is critical for ensuring the safe and secure transfer of assets within the protocol. It's important to understand its functionality and restrictions.
ODSafeManager.sol: As it manages safe movement and creation, it's essential to know how this contract operates, especially in terms of the interaction with Vault721 and safe management.
AccountingEngine.sol: AccountingEngine contract in the Open Dollar protocol manages the protocol's surplus and debt. It facilitates surplus and debt auctions, tracks debt queues, and enables settlement. Additionally, it can initiate surplus auctions, transfer surplus to designated receivers, and be disabled to handle post-settlement debt and surplus management.
CamelotRelayer.sol: The CamelotRelayer contract in the Open Dollar protocol bridges data from a CamelotRelayer TWAP pool to a standard IBaseOracle format. It converts the price into a 18-decimal format. The contract is initialized with base and quote tokens, symbol, and other parameters. It offers functions to retrieve results from the CamelotRelayer pool, with checks for data validity. This adaptation is vital for integrating price data within the Open Dollar protocol.
CamelotRelayerFactory.sol: The CamelotRelayerFactory contract in the Open Dollar protocol deploys CamelotRelayers, which fetch and transform price data. It's initialized with authorization controls and provides a method to create new CamelotRelayers with specific token pairs and periods. The contract maintains a list of deployed CamelotRelayers, essential for obtaining reliable price data within the Open Dollar system.
CamelotRelayerChild.sol: The CamelotRelayerChild contract inherits all the functionality of the "CamelotRelayer" contract and is specifically designed to be deployed by the factory. It serves as a CamelotRelayer within the Open Dollar protocol, providing price data transformation and compatibility features for base and quote tokens with specified quote periods.
UniV3Relayer.sol: The UniV3Relayer contract in the Open Dollar protocol fetches and transforms price data from a UniswapV3Pool into a standard IBaseOracle feed. It's configured with specific base and quote tokens, along with a fee tier and quote period for obtaining reliable price data. The contract ensures that the quote result is expressed in an 18 decimal format, making it compatible with the Open Dollar ecosystem.
ODGovernor.sol: The ODGovernor contract in the Open Dollar protocol is a comprehensive governance contract that incorporates various modules and extensions for decentralized decision-making. It manages voting and proposals, configures parameters like voting delay and quorum, and integrates with the Open Dollar token and a timelock contract to ensure secure and effective governance processes.
ODProxy.sol: The ODProxy contract is a simple proxy contract designed for use in the Open Dollar protocol. It allows the owner to execute functions on another contract (the target) via delegatecall. The contract enforces that only the owner can invoke these functions. It includes error handling for cases when the target address is required, the target call fails, or the sender is not the owner.
Vault721.sol: The Vault721 contract is used in the Open Dollar protocol (Version 1.5.5) to manage tradable Vaults as ERC-721 tokens. It involves governance, initialization, proxy creation, and management. Users can interact with their Vaults and trade ownership of these assets through this contract.
SAFEHandler.sol: The SAFEHandler contract is used to create a unique safe handler address for each user's SAFE within the Open Dollar protocol. It is deployed when a new SAFE is created and grants permissions to the SAFE manager to modify the SAFE associated with this contract's address.
ODSafeManager.sol: The ODSafeManager contract serves as an interface to the SAFEEngine, facilitating the management of SAFEs in the Open Dollar protocol. It provides functions to open, modify, and transfer SAFEs, as well as control access permissions. The contract also interacts with the Vault721 contract for NFT management.
BasicActions.sol: The BasicActions contract in the Open Dollar protocol facilitates the management of SAFEs. It allows users to open new SAFEs, generate and repay debt, lock and free token collateral, and manage collateralization ratios. These actions are crucial for maintaining the stability and usability of the protocol's stablecoin system.
Some privileged roles exercise powers over the Controller contract:
modifier onlyOwner() { if (msg.sender != OWNER) revert OnlyOwner(); _; }
modifier onlyGovernor() { if (msg.sender != governor) revert NotGovernor(); _; }
modifier safeAllowed(uint256 _safe) { address _owner = _safeData[_safe].owner; if (msg.sender != _owner && safeCan[_owner][_safe][msg.sender] == 0) revert SafeNotAllowed(); _; }
modifier handlerAllowed(address _handler) { if (msg.sender != _handler && handlerCan[_handler][msg.sender] == 0) revert HandlerNotAllowed(); _; }
Accordingly, I analyzed and audited the subject in the following steps;
Core Protocol Contract Overview:
I focused on thoroughly understanding the codebase and providing recommendations to improve its functionality. The main goal was to take a close look at the important contracts and how they work together in the Open Doller Protocol.
Main Contracts I Looked At
ODGovernor.sol Vault721.sol ODSafeManager.sol ODProxy.sol SAFEHandler.sol AccountingEngine.sol BasicActions.sol CamelotRelayer.sol
I started my analysis by examining the ODGovernor.sol contract. Audit the governance contract first as it has a central role in modifying protocol parameters, enabling modifications to protocol parameters and security settings. Its functions for proposal creation, voting, and quorum verification must be meticulously audited to ensure secure and accurate governance operations. Particular attention should be paid to timelock control mechanisms, role-based access controls, and protection against vulnerabilities.
Then, I turned our attention to the Vault721.sol contract second because it manages all safe ownership, transfers, and approvals via the ERC721 standard, and it interacts closely with the ODSafeManager.
Then ODSafeManager.sol contract next as it's essential to make sure it interacts securely with Vault721 and controls the creation and movement of safes within the system.
Then audit ODProxy
, which is used to restrict ownership changes to ensure protocol integrity. Security of this contract is important and SAFEHandler
as it grants permissions for ODSafeManager to make modifications to a safe. The security of this interaction is crucial but may depend on the ODSafeManager.
And AccountingEngine
contract because it deals with the financial operations of the protocol. Vulnerabilities in this contract could have significant financial implications and then BasicActions
contract, which contains core actions related to managing safes. It's important to ensure the security of these actions.Then CamelotRelayer
contract as it's responsible for fetching the market price. The reliability of price oracles is crucial for accurate protocol operation.
Documentation Review:
Then went to Review This Docs for a more detailed and technical explanation of Open Doller.
Manuel Code Review In this phase, I initially conducted a line-by-line analysis, following that, I engaged in a comparison mode.
Line by Line Analysis: Pay close attention to the contract's intended functionality and compare it with its actual behavior on a line-by-line basis.
Comparison Mode: Compare the implementation of each function with established standards or existing implementations, focusing on the function names to identify any deviations.
Architecture of the key contracts that are part of the Open Doller protocol:
This diagram illustrates the interaction among the key components of the Open Doller protocol.
The Accounting Engine plays a central role in the protocol. It receives system surplus and debt, manages debt queues, and conducts surplus and debt auctions.
Camelot Relayer: Used by the Oracle Relayer, it fetches the current market price of the system coin (OD) from a Camelot pool on the Arbitrum network. This price data is essential for various protocol operations.
UniV3 Relayer: An alternative option for fetching the OD's market price, used by the Oracle Relayer. It retrieves price data from a Uniswap V3 pool on the Arbitrum network.
The Governor (ODGovernor) is the DAO-managed contract that governs the protocol. It can modify protocol parameters, such as adding new collateral types and changing PID settings. The Governor interacts with other protocol components to propose and execute changes.
ODProxy: A more restrictive version of the DSProxy used by the Maker Protocol. It ensures that only the Vault721 contract has the ability to transfer a safe. This restriction is crucial for the security of the protocol.
Vault721: Serves as the Proxy Registry, Proxy Factory, and the ERC721 "Non-fungible Vault." It manages all safe ownership, transfers, and approvals via the ERC721 standard. It tracks proxy ownership and deploys new proxies when necessary.
SAFEHandler: Grants permission to the ODSafeManager to make modifications to a safe. A new SAFEHandler is deployed for each safe, and its address serves as a unique identifier within the SAFEEngine.
ODSafeManager: A more restrictive Safe Manager, allowing only the Vault721 contract to move a safe. It also calls Vault721 to mint a new safe when one is created.
These actions are part of the protocol and interact with various components as needed. They play a crucial role in executing various operations within the protocol.
The interaction among these components forms the backbone of the Open Dollar protocol, allowing it to manage its surplus and debt, obtain reliable price data, and govern its parameters securely. These interactions are essential for maintaining the stability and functionality of the protocol in the decentralized financial ecosystem.
Comments and Documentation: While the codebase contains comments, improving the quality of comments and documentation can enhance code readability. Consider using more explanatory comments to clarify the purpose and functionality of each section of the code. This will make it easier for other developers and auditor to understand and maintain the code.
Decimals Handling: When working with decimals, ensure that all conversions are handled accurately.For-Example The multiplier calculation in CamelotRelayer.sol could potentially benefit from additional comments and explanation for clarity.
Formal Verification: Consider a professional formal verification of the contract to identify and mitigate any security risks.
Consolidation of Related Functions:For-Exampe: Some functions like _generateDebt
and _repayDebt
in BasicActions.sol share common functionality, and combining them into a single function with parameters to specify the operation could reduce code duplication.
Overall, I consider the quality of the Open Doller codebase to be excellent. The code appears to be very mature and well-developed. We have noticed the implementation of various standards adhere to appropriately. Details are explained below:
Codebase Quality Categories | Comments |
---|---|
Code Maintainability and Reliability | The codebase demonstrates moderate maintainability with well-structured functions and comments, promoting readability. It exhibits reliability through defensive programming practices, parameter validation, and handling external calls safely. The use of internal functions for related operations enhances code modularity, reducing duplication. Libraries improve reliability by minimizing arithmetic errors. Adherence to standard conventions and practices contributes to overall code quality. However, full reliability depends on external contract implementations like openzeppelin, uniswap. |
Code Comments | The contracts have comments that are used to explain the purpose and functionality of different parts of the contracts, making it easier for developers to understand and maintain the code. The comments provide descriptions of methods, variables, and the overall structure of the contract.For-Exmaple: The code comments in the "CamelotRelayerChild" contract provide essential information. The imported interfaces and contracts are declared, and the contract's title and purpose are described. |
Documentation | The documentation of the Open Doller project is quite comprehensive and detailed, providing a solid overview of how Open Doller is structured and how its various aspects function. However, we have noticed that there is room for additional details, such as diagrams, to gain a deeper understanding of how different contracts interact and the functions they implement. With considerable enthusiasm. We are confident that these diagrams will bring significant value to the protocol as they can be seamlessly integrated into the existing documentation, enriching it and providing a more comprehensive and detailed understanding for users, developers and auditors. |
Testing | The audit scope of the contracts to be audited is 95% and it should be aimed to be 100%. |
Code Structure and Formatting | The codebase contracts are well-structured and formatted. It inherits from multiple components, adhering for-example OpenZeppelin governance standards in some contracts. The constructors initializes the contract with parameters and inherited components. Override functions for each component are provided with accompanying comments explaining their purpose. The code is organized, making it readable and maintainable. |
The analysis provided highlights several significant systemic and centralization risks present in the Open Doller protocol. These risks encompass concentration risk in ODGovernor, BasicActions, AccountingEngine risk and more, third-party dependency risk, and centralization risks arising from the existence of an โownerโ role in specific contracts. However, the documentation lacks clarity on whether this address represents an externally owned account (EOA) or a contract, warranting the need for clarification. Additionally, the absence of fuzzing and invariant tests could also pose risks to the protocolโs security.
Here's an analysis of potential systemic and centralization risks in the provided contracts:
No having, fuzzing and invariant tests could open the door to future vulnerabilities.
The AccountingEngine contract manages debt and surplus within the system, which can be a systemic risk if not executed correctly. Mishandling debt or surplus could lead to imbalances and impact the stability of the entire protocol, and this contract allows parameters to be modified. Inappropriately setting these parameters can introduce systemic risks. For example, setting incorrect surplus transfer percentages or auction bid sizes may lead to issues.
The CamelotRelayer.sol retrieves price information from an external source, the CamelotPair. Systemic risk exists if this source provides inaccurate or manipulated data, leading to incorrect price feeds and potentially affecting other parts of the protocol relying on this data and this contract converts and scales price results by manipulating decimals, if there are errors or discrepancies in this conversion, leading to incorrect pricing information and potentially impacting the entire system.
The "build" function allows users in Vault721 contract to create ODProxy contracts, but it can be abused. It does not have any checks or permissions other than verifying if a user already has an ODProxy. There is potential for abuse if users create excessive proxies, which could lead to misuse or spam.
In BasicActions.sol contract systemic risks might arise from the exit mechanisms, especially if there are discrepancies between the rates used in calculations and those from the external SAFE engine.
In AccountingEngine.sol contract allows specific accounts to be authorized and to modify parameters. Centralization risks are present if a small number of entities or individuals control these authorizations, potentially leading to centralized control over the protocol.
CamelotRelayer contract's constructor sets critical parameters like baseToken, quoteToken, baseAmount, multiplier, and quotePeriod. Centralization risks are present because these parameters are not governed by a decentralized process and are hardcoded and The getResultWithValidity
function returns false if the pool doesn't have enough history. The validity of the result depends on the pool's history and may be influenced by external factors, introducing centralization risks.
The contract UniV3Relayer.sol doesn't provide a way to modify the fee tier or other pool-related parameters after deployment, which could be problematic if adjustments are needed based on changing market conditions.
The ODGovernor.sol specifies a fixed quorum fraction (3) that is not governed or adjustable through governance processes. Centralization risks arise as this parameter cannot be modified in response to changing conditions.
The owner's address is set as immutable in ODProxy.sol contract, meaning it cannot be changed once the contract is deployed. This lack of flexibility to change ownership through a decentralized process can be a centralization risk, as there is no governance mechanism in place to handle ownership transitions and the contract does not have a mechanism for upgrades or modifications. If there are bugs or vulnerabilities discovered in the contract's code, there is no way to implement improvements without deploying an entirely new contract, which can be cumbersome and disruptive.
Properly managing these risks and implementing best practices in security and decentralization will contribute to the sustainability and long-term success of the Open Doller protocol.
In general, the Open Doller project exhibits an interesting and well-developed architecture we believe the team has done a good job regarding the code, but the identified risks need to be addressed, and measures should be implemented to protect the protocol from potential malicious use cases. Additionally, it is recommended to improve the documentation and comments in the code to enhance understanding and collaboration among developers. It is also highly recommended that the team continues to invest in security measures such as mitigation reviews, audits, and bug bounty programs to maintain the security and reliability of the project.
14 hours
#0 - c4-pre-sort
2023-10-27T01:48:34Z
raymondfam marked the issue as high quality report
#1 - pi0neerpat
2023-10-31T21:12:06Z
Excellent thank you for your thorough analysis!
#2 - c4-sponsor
2023-10-31T21:12:13Z
pi0neerpat (sponsor) acknowledged
#3 - c4-judge
2023-11-03T17:22:26Z
MiloTruck marked the issue as grade-a
#4 - c4-judge
2023-11-03T17:33:22Z
MiloTruck marked the issue as selected for report