Users can lose access to funds due to minimum withdrawal limits.

[c4-bot-6]

Lines of code

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

Vulnerability details

Impact

The InstantManager contract restricts deposits and withdrawals to certain minimum amounts. Users can deposit a minimum of 100k USDC tokens, and withdraw a minimum of 50k USDC tokens.

The issue is that the minimum withdrawal limit can lead to users losing access to part of their funds. Say a user deposits 100k USDC tokens and then later withdraws 60k USDC tokens. Now, the user only has 40kUSDC worth holdings in their account, and cannot withdraw the full amount. This is because it falls below the minimum withdrawal limit of 50k USDC tokens. The user is now stuck with 40k USDC tokens in their account, and cannot withdraw them.

The only option the user has is to deposit 100k USDC more, and then withdraw the whole 140k USDC amount. This will incur fees on the extra 100k USDC the user brings as well. Thus this is a medium severity issue.

Proof of Concept

The scenario can be recreated in the following steps:

1. User ALICE deposits 100k USDC tokens.
2. User ALICE withdraws 60k USDC tokens.
3. User ALICE tries to withdraw 40k USDC tokens. The contract reverts, as the amount is below the minimum withdrawal limit of 50k USDC tokens.

Tools Used

Manual Review

Recommended Mitigation Steps

Allow users to remove all their funds from the contract even if it is below the minimum limit. Since the protocol now uses a more liquid system such as the BUIDL token, this should be possible and should not affect the protocol's functioning.

Assessed type

Other

[c4-pre-sort]

0xRobocop marked the issue as sufficient quality report

[c4-pre-sort]

0xRobocop marked the issue as primary issue

[0xRobocop]

The worst impact is a temporary DoS over some users funds, admin can always change the limit. Leaving for sponsor review.

[cameronclifton]

This is an obvious design decision and also assumes that there is no other ways for investors to redeem OUSG/rOUSG, which is not incorrect.

[c4-sponsor]

cameronclifton (sponsor) disputed

[3docSec]

I acknowledge this behavior is a design decision.

I however would keep this as a valid medium for an audit report, because:

• there is an availability impact for users, in a condition that they did not necessarily have to purposely create for themselves
• users can decide to still withdraw for a loss in fees "for minting more to redeem all"
• the report highlights what I find to be a very reasonable mitigation - which could be the behavior users reasonably expect:

Allow users to remove all their funds from the contract even if it is below the minimum limit.

this mitigation seems feasible and difficult to exploit for systematic, abusive bypasses of minimumRedemptionAmount, because both OUSG and rOUSG have a KYC requirement on token holders

[c4-judge]

3docSec marked the issue as selected for report

[c4-judge]

3docSec marked the issue as satisfactory

[cameronclifton]

This finding is a feature suggestion to remove the minimum redemption amount. Having explicit minimum redemption and subscription amounts clearly implies that there could be conditions in which users would have a balance below the minimum and be unable to transact. Users are made well aware of these minimums. Users are also able to contact us directly should they need to redeem via other mechanisms.

Regarding the mitigation suggestion:
Allow users to remove all their funds from the contract even if it is below the minimum limit. Since the protocol now uses a more liquid system such as the BUIDL token, this should be possible and should not affect the protocol's functioning. - I don't understand how this is can be assumed to be a reasonable mitigation without knowing the reason for why the minimum subscription and redemption amounts exists.

[Henrychang26]

In setMinimumRedemptionAmount(), minimumRedemptionAmount can be set as low as FEE_GRANULARITY == 10_000.
The max amount that can potentially be locked is 9,999 which is less than $0.1 approx $0.09999

To provide more context:
In setMinimumDepositAmount(), minimumDepositAmount can also be set as low as FEE_GRANULARITY. The highest possible additional fee that may incur to put user over the minimumRedemptionAmount in order to redeem(), is 1.99%.
$0.1 x 0.0199 = $0.00199

[thebrittfactor]

Per further discussion with the sponsor team, adding this final input on their behalf for inclusion in the report:

We will not be removing minimum redemption requirement from the smart contract as there are other means in which users can redeem OUSG or rOUSG tokens from Ondo Finance.