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

5 stars 6 forks source link

Contract logic for instant redemptions is not functioning as expected due to Redemption Limit Exceedance in OUSGInstantManager Contract. #340

Closed c4-bot-7 closed 6 months ago

c4-bot-7 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

The OUSGInstantManager contract is designed to enable instant minting and redemption of OUSG and rOUSG tokens, with a focus on the redemption logic. A critical issue was identified in the enforcement of the instantRedemptionLimit by the InstantMintTimeBasedRateLimiter. This flaw significantly impacts the contract's functionality, particularly in the redemption process.

Redemption Logic Flaw: The logic for checking against the instantRedemptionLimit during redemptions is flawed. This issue prevents users from redeeming the minimum required amount of BUIDL tokens, which is assumed to be 250,000 BUIDL tokens.

Storage Variables:

Rate Limiters: The contract utilizes rate limiters to control the amount of USDC a user can redeem within a specific timeframe, aiming to prevent abuse or manipulation. However, the implementation of these limiters is flawed, leading to the observed issue.

The redemption process involves several steps, including approving the redemption amount, calling the redemption function, and handling the redemption logic within the _redeem internal function. This function performs checks on USDC and BUIDL token decimals, calls an external oracle for the current OUSG price in USDC, enforces a minimum redemption amount, and calculates redemption fees.

THE PROBLEM IS THAT: IT WILL ALWAYS REVERT!

The _redeem function interacts with two potential rate limiters: the InstantMintTimeBasedRateLimiter and an optional investorBasedRateLimiter. The logic error in _checkAndUpdateInstantRedemptionLimit prevents users from redeeming the minimum required amount of BUIDL tokens, disrupting the intended functionality. If the investorBasedRateLimiter is active, it checks and updates the redeem limit for the user with investorBasedRateLimiter.checkAndUpdateRedeemLimit(msg.sender, usdcAmountToRedeem);.

The impact of this logic issue is multifaceted. It affects the contract's functionality, user experience, and security. Users are unable to redeem tokens.

Proof of Concept

Alice, a user with a significant amount of OUSG tokens, intends to redeem a large amount of USDC exceeding the intended instantRedemptionLimit. Initially, Alice's redemption attempt fails due to the logic error in _checkAndUpdateInstantRedemptionLimit. The transaction reverts with an error message indicating the redemption amount exceeds the allowed limit[FAIL. Reason: Settlement: exceeds allowed amount].

Bob, a developer unaware of the logic error, increases the instantRedemptionLimit within the InstantMintTimeBasedRateLimiter contract to accommodate Alice's redemption request.

With the increased limit, Alice resubmits her redemption transaction. This time, the transaction succeeds because the faulty logic in _checkAndUpdateInstantRedemptionLimit no longer prevents it, even though it surpasses the originally intended limit.

If multiple users exploit the issue, the contract's USDC reserves could become depleted faster than anticipated.

Market Manipulation: Large, unexpected redemptions could lead to sudden price fluctuations of OUSG or BUIDL tokens.

function test_redeem_from_ondo_ILP_asserts_success() public {
    // Record initial USDC balance
    uint256 initialBalance = IERC20(USDC).balanceOf(BUIDL_WHALE);

    // Impersonate BUIDL_WHALE
    vm.startPrank(BUIDL_WHALE);

    // Approve BUIDL Redemptions
    IERC20(BUIDL).approve(BUIDL_REDEEMER, 10000e6);

    // Expected redemption amount
    uint256 expectedUSDC = 100e6;

    // Simulate Redemption
    IBUIDLRedeemer(BUIDL_REDEEMER).redeem(100e6);

    // Fetch final USDC balance
    uint256 finalBalance = IERC20(USDC).balanceOf(BUIDL_WHALE);

    // Assert USDC balance increase equals expected redemption amount
    assertEq(finalBalance - initialBalance, expectedUSDC);
    console.log("USDC balance change: ", finalBalance - initialBalance, expectedUSDC);
  }

Recommended Mitigation Steps

This is the though one: Review Redemption Logic: The IBUIDLRedeemer contract's redeem function likely includes checks to ensure that the redemption amount does not exceed a certain limit. This limit could be daily, total, or per user.

Assessed type

Context

0xRobocop commented 6 months ago

Invalid.

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

3docSec commented 6 months ago

OOS, as per contest README:

We are aware that KYC’d investors can DDOS the instant mint/redeem contract.

c4-judge commented 6 months ago

3docSec marked the issue as unsatisfactory: Out of scope