Ondo Finance - VAD37's results

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/rOUSG.sol#L431-L443 https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/ousgInstantManager.sol#L362-L386

Vulnerability details

Ondo Finance with 50_000 USD minimum withdrawal amount. Minimum 100_000 USD deposit. KYC user might be institution/company. Which very likely to use GnosisSafe or Timelock to manage their fund.

Signing/Executing pending transaction for 1-2 days is normal for this kind of user.

The problem with rOUSG.sol is it depend on oracle price to convert USD value into underlying investment OUSG for all operations. If oracle move while transaction is pending, it might revert.

Impact

It is possible for some special user like GnosisSafe fail to withdraw/redeem 100% of their fund. Due to oracle price movement.

Proof of Concept

rOUSG.sol is stable coin ERC20 token pegged to USD value. But rOUSG.sol only store underlying OUSG token underneath.

  /**
   * @dev rOUSG balances are dynamic and are calculated based on the accounts' shares (OUSG)
   * and the the price of OUSG. Account shares aren't
   * normalized, so the contract also stores the sum of all shares to calculate
   * each account's token balance which equals to:
   *
   *   shares[account] * ousgPrice
   */
  mapping(address => uint256) private shares;
    // ERC20 balanceOf convert user share into USD value
   function balanceOf(address _account) public view returns (uint256) {
    return
      (_sharesOf(_account) * getOUSGPrice()) /
      (1e18 * OUSG_TO_ROUSG_SHARES_MULTIPLIER);
  }

So all ERC20 function transfer, approve, transferFrom, must convert value from USD into OUSG token. Through the use of oracle.

The oracle update every 23 hours minimum.

When user want to convert their share into USD, they call rOUSG.unwarp(USD amount). This use oracle convert USD value into OUSG share then burn it.

  function unwrap(uint256 _rOUSGAmount) external whenNotPaused {
    require(_rOUSGAmount > 0, "rOUSG: can't unwrap zero rOUSG tokens");//@oracle Price ~100e18 in test file
    uint256 ousgSharesAmount = getSharesByROUSG(_rOUSGAmount);//@share = amount * 1e18 * OUSG_TO_ROUSG_SHARES_MULTIPLIER) / oraclePrice
    if (ousgSharesAmount < OUSG_TO_ROUSG_SHARES_MULTIPLIER)
      revert UnwrapTooSmall();
    _burnShares(msg.sender, ousgSharesAmount);
    ousg.transfer(
      msg.sender,
      ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER
    );
    emit Transfer(msg.sender, address(0), _rOUSGAmount);
    emit TransferShares(msg.sender, address(0), ousgSharesAmount);
  }

If user call unwarp 100_000 USD to convert their entire share of 1000 token and oracle price move down while transaction is pending.

100_000 USD now worth 1001 token. And _burnShares will revert due to not enough OUSG share.

But oracle price updated really rarely. So it must be user with transaction pending for long period of time.

This is a problem for institution user.

There is only one function to withdraw funds and it revert when over withdraw.

Tools used

manual

Recommended Mitigation Steps

Include unstake() function that use share value to withdraw funds. For accessiblity and usability.

  function unstake(uint share) external whenNotPaused {
    _burnShares(msg.sender, share);
    ousg.transfer(
      msg.sender,
      share / OUSG_TO_ROUSG_SHARES_MULTIPLIER
    );
    emit Transfer(msg.sender, address(0), getROUSGByShares(share));
    emit TransferShares(msg.sender, address(0), share);
  }

Assessed type

Timing