Ondo Finance - niser93's results

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/ousgInstantManager.sol#L335-L351

Vulnerability details

Impact

The ousgInstantManager.redeem() aims to redeem an amount of USDC against an amount of OUSG. However, it truncates decimals in order to convert OUSG (18 decimals) and OUSG price in USDC (18 decimals, obtained by the oracle) to USDC (6 decimals). So, the redeem() function ignores dust value. This behavior could lead to huge losses when it applied multiple redeem actions.

Proof of Concept

Let's describe the ousgInstantManager.redeem() function:

function redeem(
    uint256 ousgAmountIn
  )
    external
    override
    nonReentrant
    whenRedeemNotPaused
    returns (uint256 usdcAmountOut)
  {
    require(
      ousg.allowance(msg.sender, address(this)) >= ousgAmountIn,
      "OUSGInstantManager::redeem: Insufficient allowance"
    );
    ousg.transferFrom(msg.sender, address(this), ousgAmountIn);
    usdcAmountOut = _redeem(ousgAmountIn);
    emit InstantRedemptionOUSG(msg.sender, ousgAmountIn, usdcAmountOut);
  }

It takes uint256 ousgAmountIn as a parameter (using 18 decimals), and returns (transferring and burning) the corresponding value in USDC (using 6 decimals).

Let's assume that the price of OUSG is 105$ (so the value returned by the oracle is 105e18). Moreover, let's assume that the ousg.allowance(msg.sender, address(this) is enough. After the transferring of ousgAmountIn from msg.sender to address(this), redeem() function calls _redeem() function.

Let's focus on the first part of that function and ignore usdcFees:

function _redeem(
    uint256 ousgAmountIn
) internal returns (uint256 usdcAmountOut) {
    require(
      IERC20Metadata(address(usdc)).decimals() == 6,
      "OUSGInstantManager::_redeem: USDC decimals must be 6"
    );
    require(
      IERC20Metadata(address(buidl)).decimals() == 6,
      "OUSGInstantManager::_redeem: BUIDL decimals must be 6"
    );
    uint256 ousgPrice = getOUSGPrice();
    uint256 usdcAmountToRedeem = _getRedemptionAmount(ousgAmountIn, ousgPrice);

The value computed by _getRedemptionAmount() is the value of the amount of usdc to redeem. That value will be expressed using 6 decimals. Let's analyze _getRedemptionAmount() function:

function _getRedemptionAmount(
    uint256 ousgAmountBurned,
    uint256 price
) internal view returns (uint256 usdcOwed) {
    uint256 amountE36 = ousgAmountBurned * price;
    usdcOwed = _scaleDown(amountE36 / 1e18);
}

This function first computes amountE36, i.e., the value of OUSG price expressed using 36 decimals (thanks to the product operation). Then, it applies the _scaleDown() on amountE36 / 1e18, i.e., divides amountE36 before by 1e18 and then by decimalsMultiplier () (computed using ousg.decimals() - usdc.decimals(), so should be 1e12)).

In this way, it obtains usdcOwed value with 6 decimals (36 - 18 - 12 = 6). However, it truncates all less significant digits.

After we have explained how the redeem() function works, let's do a numerical example to explain how a user loses value using it. We want to underline that, thanks to ousgInstantManager.sol#L426 check, the value returned by _getRedemptionAmount() must be at least minimumRedemptionAmount (that is initially set to 50_000e6). So, if 1 OUSG has a price of 105$, we should redeem at least 477 OUSG. Let's ignore this check in the rest of the proof.

Redeeming in a single call

Let's think we want to redeem 100 OUSG (100e18). If the value returned by the oracle is 105e18, we should obtain 105*100 = 10500 USDC (10500e6).

uint256 usdcAmountToRedeem = _getRedemptionAmount(ousgAmountIn, ousgPrice);

will be called with ousgAmountIn = 100e18 and ousgPrice = 105e18. So inside _getRedemptionAmount we will have the following values: amountE36 = 10500e36 and usdcOwed = 10500e6, that is correct.

Redeeming in multiple calls

Now, let's think we want to call redeem() 19 times, and in each of these, we want to redeem 5.5555555 OUSG. In the last redeem operation, we will redeem 0.000001 OUSG. At the end, we should obtain the same value above: 105*100.55555 = 10558.333327 USDC.

The first 18 times, we redeem 5.5555555 OUSG (18*5.5555555 = 99.999999)

uint256 usdcAmountToRedeem = _getRedemptionAmount(ousgAmountIn, ousgPrice);

will be called with ousgAmountIn = 55555555e11 and ousgPrice = 105e18. So inside _getRedemptionAmount we will have the following values: amountE36 = 5833333275e29 and usdcOwed = 583333327, i.e. 583.333327e6, This value will be obtained 18 times: 583.333327e6 * 18 = 10499.999886e6

The 19th time, we redeem 0.000001 OUSG.

uint256 usdcAmountToRedeem = _getRedemptionAmount(ousgAmountIn, ousgPrice);

will be called with ousgAmountIn = 1e12 and ousgPrice = 105e18. So inside _getRedemptionAmount we will have the following values: amountE36 = 105e30 and usdcOwed = 105, i.e. 0.000105e6,

The sum of all 19 times is 10499.999886e6 + 0.000105e6 = 10499.999991e6, with a loss of about 0.000009 USDC.

Coded POC

We reported a simple code using remix.ide to show the consequence of this behavior using higher numbers:

These values are computed using the following code:

contract DustComputing {

    uint256 public immutable decimalsMultiplier = 1e12;
    uint256 public TOTAL_OUSG_AMOUNT_BURNED = 15e36;
    uint256 public TIMES_OF_REDEEM = 3.144654088709571e16;
    uint256 public SINGLE_REDEEMING_FOR_MULTIPLE_CALL = 476.9999999e18;

    function _getRedemptionAmount(
        uint256 ousgAmountBurned
    ) public view returns (uint256 usdcOwed) {
        uint256 price = 105e18;
        uint256 amountE36 = ousgAmountBurned * price;
        usdcOwed = _scaleDown(amountE36 / 1e18);
    }

    function _scaleDown(uint256 amount) public view returns (uint256) {
        return amount / decimalsMultiplier;
    }

    function compute_single_call_redeem() view public returns (uint256) {
        return _getRedemptionAmount(TOTAL_OUSG_AMOUNT_BURNED);
    }

    function compute_n_times_redeem() view public returns (uint256) {
        return _getRedemptionAmount(SINGLE_REDEEMING_FOR_MULTIPLE_CALL)*TIMES_OF_REDEEM;
    }

    function get_ousgAmountBurned_last_time() public view returns(uint256){
        return (TOTAL_OUSG_AMOUNT_BURNED - SINGLE_REDEEMING_FOR_MULTIPLE_CALL*TIMES_OF_REDEEM);
    }

    function compute_last_time_redeem() view public returns (uint256) {
        
        uint256 last_time = _getRedemptionAmount(get_ousgAmountBurned_last_time());
        return last_time;
    }

    function compute_multiple_call_redeem() view public returns (uint256) {
        return compute_n_times_redeem() + compute_last_time_redeem();
    }

    function difference_between_single_and_multiple() public view returns(uint256){
        return compute_single_call_redeem() - compute_multiple_call_redeem();
    }
}

This means that there is a loss of 15e9 USDC after the redeeming of 15e36 OUSG.

Our example uses a very huge amount of OUSG to show the loss. Due to the fact that losses are perceptible only against a very huge value to redeem and a very huge amount of redeem() calls, we report this finding as Medium. Moreover, developers wrote a function to retrieve tokens

Tools Used

Visual inspection

Recommended Mitigation Steps

Create a user's balance variable that holds dust.

Assessed type

Decimal