Ondo Finance - K42's results

Quality Assurance Report for Ondo-Finance by K42

Introduction

For Ondo-Finance: ousgInstantManager.sol, rOUSG.sol, and rOUSGFactory.sol. The aim of this report is to improve and assert the contracts' integrity, security, and optimal efficiency through examination of the codebase, functionality, and performance.

Contracts Overview

Contract 1: ousgInstantManager.sol:

• Purpose: This contract is responsible for minting and redeeming OUSG and rOUSG against USDC. It allows for optional mint and redeem fees, and implements rate limiting and pause/unpause functionality to ensure controlled and secure operations.

• Key Functionalities:

  • Minting OUSG/rOUSG: Users can mint OUSG or rOUSG by depositing USDC, subject to rate limiting and fees.
  • Redeeming OUSG/rOUSG: Users can redeem their OUSG or rOUSG for USDC, subject to rate limiting and fees.
  • Setting fees: The contract owner can set mint and redeem fees, with a maximum limit of 200 basis points.
  • Rate limiting: The contract implements time-based and investor-based rate limiting to prevent excessive minting or redeeming within specific intervals.
  • Pausing/unpausing: The contract owner can pause or unpause minting and redeeming operations to handle emergency situations or upgrades.

Contract 2: rOUSG.sol:

• Purpose: This contract is an interest-bearing ERC20 like token for OUSG. It represents the holder's share of the underlying OUSG controlled by the protocol. The contract manages the rebasing of rOUSG balances based on the OUSG price obtained from an oracle.

• Key Functionalities:

  • Wrapping/unwrapping OUSG: Users can wrap their OUSG tokens to obtain rOUSG tokens, or unwrap their rOUSG tokens to retrieve OUSG.
  • Transferring shares: Users can transfer their rOUSG shares to other addresses, subject to KYC checks.
  • Minting/burning shares: The contract allows minting and burning of rOUSG shares by authorized addresses.
  • KYC checks: The contract integrates with a KYC registry to ensure that only KYC-approved addresses can perform certain operations.
  • Pausing/unpausing: The contract can be paused or unpaused by authorized addresses to handle emergency situations or upgrades.

Contract 3: rOUSGFactory.sol

• Purpose: This contract serves as a factory for deploying upgradable instances of the rOUSG token contract. It follows the transparent proxy pattern and uses the OpenZeppelin ProxyAdmin and TransparentUpgradeableProxy contracts.

• Key Functionalities:

  • Deploying rOUSG implementation: The factory deploys the rOUSG implementation contract.
  • Deploying proxy admin: The factory deploys the ProxyAdmin contract, which is used to manage the upgradable proxy.
  • Deploying proxy: The factory deploys the TransparentUpgradeableProxy contract, which points to the rOUSG implementation and is controlled by the ProxyAdmin.
  • Transferring ownership: The factory transfers ownership of the ProxyAdmin to a designated guardian address for secure management.

Testing Criteria

• Functionality: This QA report process involved automated testing of important functions in the contracts to ensure they behave as expected. Looking for edge cases, input validation issues, and correct error handling were carefully examined. The contracts' interactions with external contracts, such as the KYC registry and the OUSG price oracle, were also tested to verify correct integration and data flow.

• Security: Security was of course top priority in this QA report process. I analysed the contracts for common vulnerabilities such as re-entrancy attacks, integer overflows/underflows, and access control issues. The use of SafeMath for arithmetic operations was verified. The contracts' best practices are good for secure contract development, such as the correct use of modifiers and the checks-effects-interactions pattern, was assessed.

• Performance: The contracts' gas usage and efficiency were evaluated, but as not in scope, just noted.

• Usability: The clarity and ease of use of the contracts' interfaces were assessed. The naming conventions, function signatures, and documentation were reviewed to ensure they are intuitive and self-explanatory. The contracts' adherence to the ERC20 standard and compatibility with existing wallets and dapps were also considered.

Findings and Observations

Contract 1: ousgInstantManager.sol:

Findings

• Finding 1: The contract uses the OpenZeppelin ReentrancyGuard to prevent reentrancy attacks. This is a good practice to ensure the contract's state is not corrupted by malicious actors attempting to recursively call the contract's functions.

• Finding 2: The contract implements role-based access control using the OpenZeppelin AccessControlEnumerable contract. This allows for granular control over who can perform certain actions, such as setting fees or pausing/unpausing the contract. However, it is important to ensure that the roles are assigned to trusted addresses and that the access control logic is properly implemented.

• Finding 3: The contract uses the SafeMath library for arithmetic operations to prevent integer overflows and underflows. This is a critical security measure, especially when dealing with user-supplied values or large numbers.

• Finding 4: The contract implements time-based and investor-based rate limiting to prevent excessive minting or redeeming within specific intervals. This helps protect against sudden spikes in demand and ensures a fair distribution of tokens. However, the rate limits should be carefully chosen based on the expected usage patterns and market conditions.

• Finding 5: The contract allows for optional mint and redeem fees, which can be set by the contract owner. While this provides flexibility, it is important to ensure that the fees are reasonable and do not create a significant burden for users. The maximum fee limit of 200 basis points is a good safeguard, but the actual fee levels should be carefully considered.

• Observations:

  • The contract has a clear separation of concerns, with distinct functions for minting, redeeming, setting fees, and managing rate limits. This makes the contract easier to understand, maintain, and audit.
  • The contract uses the nonReentrant modifier on the mint and redeem functions to prevent reentrancy attacks. This is a good practice, but it is important to ensure that the modifier is applied consistently and correctly.
  • The contract uses the whenNotPaused modifier on the mint and redeem functions to prevent usage when the contract is paused. This is a useful feature for handling emergency situations or upgrades, but it should be used judiciously to avoid disrupting user experience.
  • The contract emits events for important actions such as minting, redeeming, and setting fees. This provides transparency and allows for easy monitoring and auditing of the contract's operations.

Security Vulnerabilities

• Vulnerability 1: The retrieveTokens function allows the contract owner to retrieve any token from the contract. While this can be useful for recovering stuck tokens, it also poses a risk if the contract owner's private key is compromised. An attacker could use this function to drain the contract of all its tokens. Add additional safeguards, such as a time lock or a multi-sig requirement, to mitigate this risk.

• Vulnerability 2: The setOracle function allows the contract owner to change the address of the OUSG price oracle. If the oracle is compromised or provides inaccurate price data, it could lead to incorrect minting or redeeming amounts. Consider implementing a mechanism to verify the integrity of the oracle data, such as using multiple oracles or a decentralized oracle network.

• Vulnerability 3: The contract relies on the accuracy of the decimalsMultiplier value, which is calculated based on the difference between the decimals of OUSG and USDC. If the decimals of either token change in the future, it could lead to incorrect calculations. Consider adding a function to update the decimalsMultiplier value if needed, with appropriate access control and validation.

Contract 2: rOUSG.sol:

Findings

• Finding 1: The contract uses a shares mapping to track each account's share of the total OUSG supply. This allows for efficient tracking of ownership and eliminates the need for expensive storage reads and writes. However, it is important to ensure that the share calculations are accurate and consistent with the actual OUSG balances.

• Finding 2: The contract implements KYC checks using the KYCRegistryClientUpgradeable contract. This ensures that only KYC-approved addresses can perform certain operations, such as transferring tokens or minting/burning shares. However, the effectiveness of the KYC checks depends on the quality and reliability of the KYC registry. It is important to regularly review and update the KYC registry to ensure compliance with the latest regulations and best practices.

• Finding 3: The contract uses the _beforeTokenTransfer hook to perform KYC checks on the sender and recipient addresses before any token transfer. This is a good practice to enforce compliance and prevent unauthorized transfers. However, it is important to ensure that the KYC checks do not introduce significant gas overhead or impact the performance of token transfers.

• Finding 4: The contract allows for burning of rOUSG shares by authorized addresses, such as the contract owner or a designated burner role. While this can be useful for managing the token supply or handling exceptional situations, it is important to ensure that the burning functionality is not abused or used to unfairly manipulate the token price. Consider implementing additional safeguards, such as a time lock or a multi-sig requirement, to prevent unauthorized burning.

• Observations:

  • The contract effectively manages the rebasing of rOUSG balances based on the OUSG price obtained from an oracle. This ensures that rOUSG holders maintain their proportional ownership of the underlying OUSG, even as the total supply changes.
  • The contract uses the _transfer and _transferShares functions to handle the low-level details of token transfers, such as updating balances and emitting events. This keeps the high-level transfer functions (transfer, transferFrom) simple and readable.
  • The contract implements the pause and unpause functions to allow authorized addresses to pause or unpause the contract in case of emergency situations or upgrades. This is a useful feature, but it should be used sparingly to avoid disrupting user experience.

Security Vulnerabilities

• Vulnerability 1: The contract does not have any access control on the wrap and unwrap functions, allowing any address to wrap or unwrap OUSG. While this is intended behaviour, it could potentially be exploited by malicious actors to manipulate the rOUSG supply or price. Consider adding a mechanism to limit the maximum amount of OUSG that can be wrapped or unwrapped in a single transaction, or implementing a fee structure to discourage excessive wrapping/unwrapping.

• Vulnerability 2: The contract relies on the accuracy of the OUSG price obtained from the oracle. If the oracle is compromised or provides inaccurate price data, it could lead to incorrect rebasing of rOUSG balances. Consider implementing additional safeguards, such as using multiple oracles or a decentralized oracle network, to ensure the integrity of the price data.

• Vulnerability 3: The contract allows the contract owner to change the address of the KYC registry using the setKYCRegistry function. If the new KYC registry is not properly vetted or is compromised, it could allow unauthorized addresses to bypass the KYC checks. Consider implementing a multi-sig requirement or a time lock for changing the KYC registry address to prevent single points of failure.

Contract 3: rOUSGFactory.sol:

Findings

• Finding 1: The contract uses the OpenZeppelin ProxyAdmin and TransparentUpgradeableProxy contracts for deploying upgradable instances of the rOUSG token contract. This well-established pattern for implementing upgradable contracts, allows for flexibility in updating the contract logic without changing the contract address or state.

• Finding 2: The contract transfers ownership of the ProxyAdmin contract to a designated guardian address after deployment. This is a good practice to ensure that the contract is not controlled by a single entity and reduces the risk of unauthorized upgrades. However, it is important to ensure that the guardian address is a secure, multi-sig wallet to minimize the risk of a single point of failure.

• Finding 3: The contract uses the onlyGuardian modifier to restrict access to certain functions, such as deployRebasingOUSG and multiexcall. This ensures that only the designated guardian address can perform these critical operations. However, it is important to ensure that the guardian address is properly protected and that there are clear policies and procedures for using the guardian role.

• Observations:

  • The contract follows a standard pattern for deploying upgradable contracts using the OpenZeppelin libraries. This reduces the risk of implementation errors and makes the contract easier to audit and maintain.
  • The contract emits a rOUSGDeployed event when a new rOUSG instance is deployed, providing transparency and allowing for easy monitoring of the contract's operations.
  • The contract includes a multiexcall function that allows the guardian to perform multiple contract calls in a single transaction. While this can be useful for batch operations or complex workflows, it is important to ensure that the calls are properly validated and do not introduce any unintended side effects.

Security Vulnerabilities

• Vulnerability 1: The contract does not have a mechanism to prevent the guardian from being changed to an invalid or malicious address. If the guardian address is compromised or set to an invalid address, it could lead to a loss of control over the contract and potentially allow for unauthorized upgrades or other malicious actions. Suggestion: Implement a multi-sig requirement or a time lock for changing the guardian address to prevent single points of failure.

• Vulnerability 2: The contract does not have any checks to ensure that the kycRegistry, ousg, or oracle addresses provided to the deployRebasingOUSG function are valid or trusted. If any of these addresses are set to malicious contracts, it could compromise the security and integrity of the deployed rOUSG instance. Suggestion: Add additional validation checks or using a whitelist of trusted addresses to ensure the safety of the deployed contracts.

Recommendations

Contract 1: ousgInstantManager.sol:

• Recommendation 1: Implement additional safeguards for the retrieveTokens function, such as a time lock or a multi-sig requirement, to prevent unauthorized token retrieval in case of a compromised contract owner.

• Recommendation 2: Implement a mechanism to verify the integrity of the OUSG price oracle data, such as using multiple oracles or a decentralized oracle network, to reduce the risk of price manipulation.

• Recommendation 3: Add a function to update the decimalsMultiplier value if needed, with appropriate access control and validation, to ensure correct calculations if the token decimals change in the future.

• Recommendation 4: Review and adjust the mint and redeem fee levels based on market conditions and user feedback for a fair and competitive fee structure.

Contract 2: rOUSG.sol:

• Recommendation 1: Consider adding a mechanism to limit the maximum amount of OUSG that can be wrapped or unwrapped in a single transaction, or implementing a fee structure to discourage excessive wrapping/unwrapping and prevent potential price manipulation.

• Recommendation 2: Implement additional safeguards for the OUSG price oracle, such as using multiple oracles or a decentralized oracle network, to ensure the integrity of the price data and reduce the risk of incorrect rebasing.

• Recommendation 3: Implement a multi-sig requirement or a time lock for changing the KYC registry address to prevent single points of failure and ensure proper vetting of new KYC registries.

• Recommendation 4: Review and update the KYC registry to ensure compliance with the latest regulations and best practices, maintaining the effectiveness of the KYC checks.

Contract 3: rOUSGFactory.sol

• Recommendation 1: Implement a multi-sig requirement or a time lock for changing the guardian address to prevent single points of failure and ensure proper control over the contract.

• Recommendation 2: Add validation checks or use a whitelist of trusted addresses for the kycRegistry, ousg, and oracle addresses provided to the deployRebasingOUSG function to ensure the safety and integrity of the deployed rOUSG instances.

• Recommendation 3: Implement a mechanism to verify the correctness and safety of the calls made through the multiexcall function, such as input validation, gas limits, or a whitelist of allowed function signatures, to prevent unintended side effects or malicious actions.

Conclusion

The 3 contracts implement good security measures = re-entrancy guards, access control, and SafeMath for arithmetic operations. They also provide good features such as rate limiting, KYC checks, and upgradability through the use of the proxy contracts. By addressing the identified quality assurance issues above, and continuing to follow best practices, Ondo-Finance can be confident that the OUSGInstantManager, ROUSG, and ROUSGFactory contracts provide a secure, reliable, and efficient platform for managing minting, redeeming, and rebasing of the OUSG and rOUSG tokens.