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

5 stars 6 forks source link

Lack of Slippage Protection when minting or redeeming OUSG in `OUSGInstantManager` #100

Open c4-bot-1 opened 6 months ago

c4-bot-1 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/ousgInstantManager.sol#L278-L324 https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/ousgInstantManager.sol#L388-L456

Vulnerability details

Issue Description

Users can mint OUSG/ROUSG tokens by calling mint or mintRebasingOUSG functions respectively and by providing a certain amount of USDC tokens and they can also redeem their USDC afterwards by calling redeem or redeemRebasingOUSG.

For each of these operation (mint or redeem) the current OUSG/USD price is fetched from the oracle and is used as the main exchange rate between OUSG and USDC, this is done in both minting and redeeming functions through invoking getOUSGPrice() function and then converting between OUSG/USDC using _getMintAmount (when minting) or _getRedemptionAmount (when redeeming).

As we all know token prices can fluctuate between the moment a tx is submit on chain and when its executed, for example tx could stay pending for relatively long time in mempool. Knowing this and the fact that neither the minting or redeeming functions implement some sort of slippage protection mechanism (like min amount out or tx deadline), users will be at risk of losing a portion of the token they are expecting to receive (either OUSG/ROUSG when minting or USDC when redeeming) due to slippage occurring on OUSG price fluctuations.

NOTE: This issue is different from the out of scope issues related to OUSG price which are more related to price manipulation or unrealistic price mouvements, this issue is related to normal OUSG price fluctuation in the market when a user submit a mint/redeem tx.

To illustrate this issue, consider the following scenario:

If Bob had known that the OUSG price had dropped, he would have waited until the price increased again to redeem his tokens.

And the same issue can occur when minting, in that case if OUSG price increase while users tx is pending they will receive less OUSG tokens that expected.

Impact

Users at risk of losing funds when minting or redeeming OUSG/ROUSG from OUSGInstantManager due OUSG price change which result in important slippage.

Tools Used

Manual review, VS Code

Recommended Mitigation

Minting (mint, mintRebasingOUSG) and redeeming (redeem or redeemRebasingOUSG) functions in OUSGInstantManager should include slippage protection parameters provided by the users (either minimum amount out or a tx deadline).

Assessed type

Context

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as duplicate of #250

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as duplicate of #156

c4-judge commented 6 months ago

3docSec marked the issue as satisfactory

c4-judge commented 6 months ago

3docSec changed the severity to QA (Quality Assurance)

c4-judge commented 6 months ago

3docSec marked the issue as grade-b