c4-bot-7 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts%2Fousg%2FousgInstantManager.sol#L388-L400 https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts%2Fousg%2FousgInstantManager.sol#L699-L705

Vulnerability details

Users first deposit/mint USDC and get an equal amount of OUSG then can be wrapped into rOUSGto earn rebase interests. However when the user wants to redeem the OUSG for USDC due to the difference in magnitude between amountE36 and decimalsMultiplier rounding issue occurs which leads to loss of funds for the user.

the amountE36represents the product of the ousgAmountBurned and price, which is relatively small compared to the large value of decimalsMultiplier.

Impact

Financial loss for the user

Proof of Concept

Users redeem by calling redeem

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts%2Fousg%2FousgInstantManager.sol#L335-L351

  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);
  }
``
Which inturn calls `_redeem` with the `OUSG` amount being withdrawn by the user 
https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts%2Fousg%2FousgInstantManager.sol#L388-L400
```solidity
  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);

Then the _getRedemptionAmount calculates the amount of USDC the user recieves. https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts%2Fousg%2FousgInstantManager.sol#L699-L705

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

SupposeousgAmountBurnedis 1,000,000 OUSG tokens and price is $0.001 USDC per OUSG token.

Calculate the amount in E36 (E36 denotes 10^36):

amountE36 = ousgAmountBurned * price
          = 1,000,000 * 0.001
          = 1,000`

scale down amountE36by decimalsMultiplier, which is 1 trillion:

scaledAmount = amountE36 / decimalsMultiplier
             = 1,000 / 1,000,000,000,000
             = 0

decimalsMultiplier ` is calculated as follows

decimalsMultiplier =
      10 **
        (IERC20Metadata(_ousg).decimals() - IERC20Metadata(_usdc).decimals());

decimalsMultiplier = 10 ** (IERC20Metadata(_ousg).decimals() - IERC20Metadata(_usdc).decimals())
                   = 10 ** (18 - 6)
                   = 10 ** 12
                   = 1,000,000,000,000

In this example, after scaling down, the result is rounded down to 0 due to the significant difference in magnitudes between amountE36 and decimalsMultiplier.

scaledAmount = amountE36 / decimalsMultiplier
             = 1,000 / 1,000,000,000,000
             = 0

Which indicates smaller values will be rounded down to zero leading to loss of precision in the calculation

Tools Used

Manual Review

Recommended Mitigation Steps

consider using fixed-point arithmetic or external library for precise calculations.

Assessed type

Math

c4-pre-sort commented 7 months ago

0xRobocop marked the issue as duplicate of #246

0xRobocop commented 7 months ago

0.001 USDC per OUSG is not realistic.

c4-judge commented 7 months ago

3docSec changed the severity to QA (Quality Assurance)