Ondo Finance - Breeje's results

Institutional-Grade Finance. On-Chain. For Everyone.

General Information

Platform: Code4rena

Start Date: 01/09/2023

Pot Size: $36,500 USDC

Total HM: 4

Participants: 70

Period: 6 days

Judge: kirk-baird

Id: 281

League: ETH

Ondo Finance

Findings Distribution

Researcher Performance

Rank: 36/70

Findings: 2

Award: $25.93

QA:
grade-b
Analysis:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Awards

7.08 USDC - $7.08

Labels

bug
grade-b
QA (Quality Assurance)
sufficient quality report
Q-03

External Links

QA Report

Low Risk Issues

CountExplanation
[L-01]No checks to confirm that the gas paid by user is enough
[L-02]In case DestinationBridge is paused before SourceBridge, User can lose funds
[L-03]No incentive for approvers to approve can lead to issues in long term
[L-04]Possible Reorg situation in rUSDYFactory
Total Low Risk Issues4

[L-01] No checks to confirm that the gas paid by user is enough

Proof of Concept

In SourceBridge contract, When any user calls burnAndCallAxelar:

  1. TOKENs are burned from caller.
  2. Gas Fees passed by user is paid through payNativeGasForContractCall.
  3. callContract call is made to transfer the token cross chain.

  function burnAndCallAxelar( 
    uint256 amount,
    string calldata destinationChain
  ) external payable whenNotPaused {
    string memory destContract = destChainToContractAddr[destinationChain];

    if (bytes(destContract).length == 0) {
      revert DestinationNotSupported();
    }

    if (msg.value == 0) {
      revert GasFeeTooLow();
    }

    TOKEN.burnFrom(msg.sender, amount);

    bytes memory payload = abi.encode(VERSION, msg.sender, amount, nonce++);

    _payGasAndCallContract(destinationChain, destContract, payload);
  }

  function _payGasAndCallContract(
    string calldata destinationChain,
    string memory destContract,
    bytes memory payload
  ) private {
    GAS_RECEIVER.payNativeGasForContractCall{value: msg.value}(
      address(this),
      destinationChain,
      destContract,
      payload,
      msg.sender
    );

    AXELAR_GATEWAY.callContract(destinationChain, destContract, payload);
  }

Here, there is no validation that the Gas Fees paid by user is sufficient to transfer the token cross chain or not.

Impact

There are transaction won't be successful at the destination chain because of lack of gas fees paid. User might need to manually call

As per Axelar Docs,

An application that wants Axelar to automatically execute contract calls on the destination chain needs to do three things:

  1. Estimate the gasLimit that the contract call will require on your executable contract on the destination chain.
  2. Call the estimateGasFee method to get the sourceGasFee in the desired gas-payment token on the destination chain.
  3. Pay the AxelarGasService smart contract on the source chain in the amount calculated in step 2.

Given that the extra cost will be refunded to user, a mapping state variable can be added which can save the estimate gas fees it will cost for different chains which owner can update as well. And then keep a require check to make sure that user are passing enough gas to execute the transaction seamlessly.

[L-02] In case DestinationBridge is paused before SourceBridge, User can lose funds

Proof of Concept

When user wants to let's say transfer x amount of tokens from Chain A to Chain B, below is the sequence of events that will occur:

  1. User calls burnAndCallAxelar on SourceBridge contract.
  2. Tokens of user will be burnt.
  3. _execute on destination contract will be called.
  4. Once user gets enough approvals, tokens will be minted on destination chain.

Issue here can arise during emergency, if DestinationBridge is paused before SourceBridge. In this case, user's token will be burnt in the source contract but user will not be able to mint the tokens in the destination contract.

Impact

Loss of funds for User.

Always makes sure to pause all the source contract first during emergency before destination contracts.

[L-03] No incentive for approvers to approve can lead to issues in long term

On destination chain, the last approver will be required to pay extra gas fees to perform extra operations of Updating mint limit, minting the tokens to sender etc.

Here, approver has no incentive to do it and gas fees can be quite high in case the chain is ethereum.

Hence, it is recommended to have an additional function which should be called by user to mint the tokens and perform the operation in _mintIfThresholdMet function. Or another option is to introduce a fee on transfer of tokens which will be distributed to approvers to compensate for their gas fees cost.

[L-04] Possible Reorg situation in rUSDYFactory

The deployrUSDY function deploys a new rUSDY contract using the create.

Some of the chains like Arbitrum and Polygon are suspicious of the reorg attack in case contracts are not deployed with Deployed via create2 with salt.

File: rUSDYFactory.sol

  function deployrUSDY(
    address blocklist,
    address allowlist,
    address sanctionsList,
    address usdy,
    address oracle
  ) external onlyGuardian returns (address, address, address) {
    rUSDYImplementation = new rUSDY();
    rUSDYProxyAdmin = new ProxyAdmin();
    rUSDYProxy = new TokenProxy(
      address(rUSDYImplementation),
      address(rUSDYProxyAdmin),
      ""
    );

So it is recommended to deploy via create2 with salt only.

#0 - c4-pre-sort

2023-09-08T08:37:16Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-09-21T10:13:08Z

kirk-baird marked the issue as grade-b

Findings Information

Awards

18.8458 USDC - $18.85

Labels

analysis-advanced
grade-b
sufficient quality report
A-04

External Links

Analysis

Summary

CountTopic
1Introduction
2Audit Approach
3Architecture Recommendation
4Centralization Risk
5Code Review Insights
6Time Spent

Introduction

This technical analysis report focuses on the underlying smart contracts of Ondo Finance consisting of the rebasing variant of USDY token and bridging tokens using Axelar Network. This report explores various contract categories, including Rebase token itself, Bridging component and oracle implementation.

Audit Approach

  1. Initial Scope and Documentation Review: Thoroughly went through the Contest Readme File and project documentation to understand the protocol's objectives and functionalities.

  2. High-level Architecture Understanding: Performed an initial architecture review of the codebase by going through all files without going into function details.

  3. Test Environment Setup and Analysis: Set up a test environment and execute all tests. Additionally, use Static Analyzer tools like Slither to identify potential vulnerabilities.

  4. Comprehensive Code Review: Conducted a line-by-line code review focusing on understanding code functionalities.

    • Understand Codebase Functionalities: Began by going through the functionalities of the codebase to gain a clear understanding of its operations.
    • Analyze Value Transfer Functions: Adopted an attacker's mindset while inspecting value transfer functions, aiming to identify potential vulnerabilities that could be exploited.
    • Identify Access Control Issues: Thoroughly examined the codebase for any access control problems that might allow unauthorized users to execute critical functions.
    • Evaluate Function Execution Order: Checked random sequence of function executions to ensure the protocol's logic cannot be disrupted by changing the order of calls.
    • Assess State Variable Handling: Identified state variables and assess the possibility of breaking assumptions to manipulate them into exploitable states, leading to unintended exploitation of the protocol's functionality.
  5. Report Writing: Write Report by compiling all the insights I gained throughout the line by line code review.

Architecture Recommendation

The following architecture improvements and feedback could be considered:

  • On destination chain, approvers will be required to pay extra gas fees to perform extra operations of Updating mint limit, minting the tokens to sender etc. Here, approvers has no incentive to do it and gas fees can be quite high in case the chain is ethereum. So it is recommended to have an additional function which should be called by user to mint the tokens and perform the operation in _mintIfThresholdMet function. Or another option is to introduce a fee on transfer of tokens which will be distributed to approvers to compensate for their gas fees.

Centralization Risk

Role of Admin is very crucial in almost every contract. Here are the Centralization Risk which Users needs to be aware of before using the smart contracts in scope:

  • If threshold is set too high, User can potentially never mint their tokens at destination chain.
  • Owner can potentially remove all the approvers anytime which can result in User not able to mint tokens at destination chain.
  • Owner can rescue any fund on DestinationBridge contract.
  • Owner can set mintLimit as very low value to force DoS on DestinationBridge contract.
  • LIST_CONFIGURER_ROLE can add any address to blocklist, SanctionsList or Allowlist.
  • PAUSER_ROLE can pause all the operations of rUSDY token anytime.

Code Review Insights

Bridge Component

Contracts:

  1. SourceBridge:
  2. DestinationBridge
  3. MintTimeBasedRateLimiter

Insights / Critical Points

  • No checks on whether user provided sufficient amount of gas to execute the function at destination chain or not.
  • In case of emergency, if DestinationBridge is paused before SourceBridge, User can potentially burn their tokens while will fail to mint it back on destination chain.
  • Lots of centralization risk in DestinationBridge contract which can lead to user not able to mint the token at destination chain.

rUSDY token Component

Contracts:

  1. rUSDYFactory
  2. rUSDY
  3. RWADynamicOracle

Insights / Critical Points

  • 1 Wei of USDY is equivalent to 10_000 rUSDY shares.
  • transferFrom function reduces allowances even if _sender is msg.sender
  • Any address can be added to Blocklist, Allowlist and SanctionsList in rUSDY token by LIST_CONFIGURER_ROLE which can impact their ability to use their funds.
  • User can wrap or unwrap their USDY to or from rUSDY.
  • Oracle returns the price of USDY which is used to calculate totalSupply.
  • If a range is modified, the previous range's closing price will become outdated for all subsequent ranges that are set.

Time Spent

Total Number of Hours10

Time spent:

10 hours

#0 - c4-pre-sort

2023-09-08T14:47:14Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2023-09-24T07:05:47Z

kirk-baird marked the issue as grade-a

#2 - c4-judge

2023-09-24T07:13:31Z

kirk-baird marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter