code-423n4 / 2024-03-ondo-finance-findings

5 stars 6 forks source link

The contract assumes that the price of USDC is always $1.00 which will be a problem if the price depegs #297

Closed c4-bot-8 closed 3 months ago

c4-bot-8 commented 3 months ago

Lines of code

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

Vulnerability details

The ousgInstantManager is used to mint or redeem OUSG/rOUSG using USDC on a 1:1 ratio. To calculate the amount of OUSG we will mint or redeem we use the _getMintAmount() and _getRedemptionAmount() functions.

function _getMintAmount(
    uint256 usdcAmountIn,
    uint256 price
  ) internal view returns (uint256 ousgAmountOut) {
    uint256 amountE36 = _scaleUp(usdcAmountIn) * 1e18;
    ousgAmountOut = amountE36 / price;
  }

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

So for example if we want to mint and use 105 USDC worth $1 and the price of OUSG is 105 the calculation will be:

(105e6 1e12) 1e18 / 105e18 = 1e18

However the problem here is that the USDC is always priced at 1$ because we are only using the amount of USDC being sent without the current price. This will create problems when the price of USDC depegs which happened for example last year.

Users will be able to use USDC to mint OUSG as if the price was 1$ even though the price can be lower allowing the users to profit from this. Or if the price goes above 1$, users will receive the amount of USDC when redeeming as if it was 1$ even though it is worth more.

Impact

If the price of USDC depegs the users will be able to mint OUSG using a discounted price or they will be able to receive USDC at a higher price when redeeming allowing them to profit from events like this.

Proof of Concept

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

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

function _getMintAmount(
    uint256 usdcAmountIn,
    uint256 price
  ) internal view returns (uint256 ousgAmountOut) {
    uint256 amountE36 = _scaleUp(usdcAmountIn) * 1e18;
    ousgAmountOut = amountE36 / price;
  }

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

As you can see here the amount of OUSG that the user will mint/redeem uses the amount of USDC sent which is always priced at 1$ and it doesnt check the current price.

Tools Used

Manual Review

Recommended Mitigation Steps

The easiest way to fix this is to get the USDC price from an oracle and revert if it is for example < 0.99$ or > 1.01$

Assessed type

Other

0xRobocop commented 3 months ago

The issue seems valid. The oracle used for comparison is SHV / USD from Chainlink, found in RWAOracleExternalComparisonCheck.sol.

c4-pre-sort commented 3 months ago

0xRobocop marked the issue as sufficient quality report

c4-pre-sort commented 3 months ago

0xRobocop marked the issue as primary issue

c4-pre-sort commented 3 months ago

0xRobocop marked the issue as high quality report

c4-sponsor commented 3 months ago

cameronclifton (sponsor) acknowledged

cameronclifton commented 3 months ago

This is fair and not explicitly called out as a known issue in the readme. I'm on the fence on whether we will employ this mitigation.

c4-judge commented 3 months ago

3docSec changed the severity to 3 (High Risk)

c4-judge commented 3 months ago

3docSec marked the issue as satisfactory

c4-judge commented 3 months ago

3docSec marked issue #278 as primary and marked this issue as a duplicate of 278