Ondo Finance - Tigerfrake's results

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/rOUSG.sol#L624-L640

Vulnerability details

Summary

The ROUSG.burn() function allows admin to burn rOUSG tokens from any _account thereby getting OUSG in return. The ROUSG.unwrap() on the other hand is called by users to unwrap their rOUSG tokens.

However, these functions do not have any type of slippage control since the amount of tokens received by the user is determined by an interaction with an oracle, meaning that the amount received in return may vary indefinitely while the request is waiting to be executed.

Also the users will have no defense against price manipulation attacks, if they were to be found after the protocol's deployment.

Proof of Concept

Instance 1: ROUSG.burn()

• https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/rOUSG.sol#L624-L640

  function burn(
    address _account,
    uint256 _amount
  ) external onlyRole(BURNER_ROLE) {
    uint256 ousgSharesAmount = getSharesByROUSG(_amount);
    if (ousgSharesAmount < OUSG_TO_ROUSG_SHARES_MULTIPLIER)
      revert UnwrapTooSmall();


    _burnShares(_account, ousgSharesAmount);


    ousg.transfer(
      msg.sender,
      ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER
    );
    emit Transfer(address(0), msg.sender, getROUSGByShares(ousgSharesAmount));
    emit TransferShares(_account, address(0), ousgSharesAmount);
  }

The ousgSharesAmount to be burned is determined by the getSharesByROUSG() function of the same contract:

• https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/rOUSG.sol#L363-L368

  function getSharesByROUSG(
    uint256 _rOUSGAmount
  ) public view returns (uint256) {
    return
      (_rOUSGAmount * 1e18 * OUSG_TO_ROUSG_SHARES_MULTIPLIER) / getOUSGPrice();
  }

As can be observed, this function uses oracle interaction through the division by getOUSGPrice(). The getOUSGPrice() function queries the external oracle for the asset price:

• https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/rOUSG.sol#L378-L380

  function getOUSGPrice() public view returns (uint256 price) {
    (price, ) = oracle.getPriceData();
  }

To determine the amount of OUSG to be received after share burning, the ousgSharesAmount calculated above is divided by OUSG_TO_ROUSG_SHARES_MULTIPLIER:

• https://github.com/code-423n4/2024-03-ondo-finance/blob/be2e9ebca6fca460c5b0253970ab280701a15ca1/contracts/ousg/rOUSG.sol#L634-L637

    ousg.transfer(
      msg.sender,
      ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER
    );

This means that the user has no way to predict how many OUSG they will get back at the moment of burning shares, as the price could be updated while the request is in the mempool.

Instance 2: ROUSG.unwrap()

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

  function unwrap(uint256 _rOUSGAmount) external whenNotPaused {
    require(_rOUSGAmount > 0, "rOUSG: can't unwrap zero rOUSG tokens");
    uint256 ousgSharesAmount = getSharesByROUSG(_rOUSGAmount);
    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);
  }

ousgSharesAmount is detamined by calling getSharesByROUSG():

  function getSharesByROUSG(
    uint256 _rOUSGAmount
  ) public view returns (uint256) {
    return
      (_rOUSGAmount * 1e18 * OUSG_TO_ROUSG_SHARES_MULTIPLIER) / getOUSGPrice();
  }

which also uses getOUSGPrice() in division as above. This invokes the external oracle to get the asset price:

  function getOUSGPrice() public view returns (uint256 price) {
    (price, ) = oracle.getPriceData();
  }

And finally to determine the amount of OUSG to be received after unwrapping, the ousgSharesAmount calculated above is divided by OUSG_TO_ROUSG_SHARES_MULTIPLIER:

    ousg.transfer(
      msg.sender,
      ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER
    );

Similarly, the user has no way to predict how many OUSG they will get back at the moment of unwrapping, as the price could be updated while the request is in the mempool.

Tools Used

Manual Review

Recommended Mitigation Steps

Add additional parameter that could be added in these functions, to decide the minimum amount of tokens to be received, with a relative check after burning/unwrapping.

Assessed type

Oracle