fee in completeRedemptions() can be set to any value.

[code423n4]

Lines of code

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

Vulnerability details

Impact

When redemptions are completed in the completeRedemptions() function the fee extracted can be set to any value. An amount of centralization over the usual is to be expected in a protocol that deals with RWAs and requires KYC but being able to set an arbitrary fee on a per-redemption basis is unnecessary, dangerous, and could harm Ondo's reputation.

Proof of Concept

The risk lies in both the reputational damage to Ondo when users realize that a fee can be set to any value on each redemption. Users in the DeFi space have a high standard when it comes to trustlessness and will no look favorably on unnecessary centralization that can harm users.

There is an additional risk where the manager sets the fee to a very high number by misstake since there are no checks, this would also harm Ondo's reputation and users.

Tools Used

Manual Review

Recommended Mitigation Steps

Create a new variable redeemFee that has an upper limit that can be set by the manager. If the goal of having a fee parameter is to refund gas used, calculating gas used by using the gasLeft() function is appropriate.

[c4-judge]

Duplicate of https://github.com/code-423n4/2023-01-ondo-findings/issues/281

[c4-judge]

trust1995 marked the issue as grade-c

[c4-sponsor]

tom2o17 marked the issue as sponsor acknowledged

[tom2o17]

We have to assume that the admin of managerAdmin is not a malicious actor, otherwise there are many ways which a malicious admin can "Rug" users.

Re mitigation steps: It makes no sense to have a settable upper fee limit that is set by managerAdmin as this does not mitigate the issue outlined here all it will require is that the managerAdmin set the upper bound to "any value" prior to placing "any value" in the call to completeRedemption

[c4-judge]

trust1995 marked the issue as grade-b