search

code-423n4 / 2023-01-ondo-findings

0 stars 0 forks source link

Users have no guarantee of a minimum amount received when minting and redeeming #304

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L707-L727 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L241-L269

Vulnerability details

Impact

It is a widely accepted practice in DeFi to offer users the ability to specify the least favorable exchange they are willing to accept. Since CASH tokens are not received in the same transaction it is not possible to create a wrapper that checks if they received amount is enough. Instead, the protocol can set any exchange rate or even wipe the user's mint balance with setPendingMintBalancd and their redemption balance with setPendingRedemptionBalance

This is a design choice that I believe puts users at risk and leads to uncertainty that users might not be comfortable with.

Changes could be made that still give Ondo the centralization access it needs to offer a product based on RWAs with KYC without features that can be dangerous to users and Ondo's reputation.

Proof of Concept

Any exchange rate can be set and users are forced to accept it when they call claimMint(). Similarly, they are forced to accept the exchange rate implied by the collateralAmountToDist set in the completeRedemptions() by a MANAGER_ADMIN.

Users' balances can also be wiped or arbitrarily changed with setPendingMintBalancd() setPendingRedemptionBalance()

Tools Used

Manual Review

Recommended Mitigation Steps

Add functionality where users can specify the minimum acceptable CASH received and if the exchange rate is too low they can simply refund themselves. The protocol will have to hold on to the deposited USDC and only send it to 'feeReceiver' when users receive CASH that is at or above the minimum they set.

A similar solution is necessary when refunding. The completeRedemptions() function should automatically refund instead of processing users' redemption when the amount is less than the minimum set by the user.

This would offer a superior UI where users are certain that they will either receive the minimum amount or that they will be refunded, this is in line with how other decentralized protocols function.

Ondo can retain the necessary centralization while providing safety and certainty to users.

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Out of scope

c4-sponsor commented 1 year ago

tom2o17 marked the issue as sponsor acknowledged