Not sure what the use case of setPendingMintBalance() should be, but it can cause admins to drain the whole balance, nullify the user's funds or break the protocol. This can be caused on purpose, accidentally or through an attacker stealing MANAGER_ADMIN's private key.
Proof of Concept
If MANAGER_ADMIN calls
setPendingMintBalance(userAddr, 1, 10,0)
, it would cause userAddr to lose all the collateral deposited.
If MANAGER_ADMIN calls
setPendingMintBalance(managerAddr, 1, 0, max)
, then managerAddr can mint any amount of cash and then redeem it to take all the collateral.
If MANAGER_ADMIN calls
setPendingMintBalance(someAddress, 1, 1, 2)
, then the address can withdraw more tokens than he deposited and thus taking someone else's collateral out of the protocol (stealing someone else's funds).
Lines of code
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L336-L350
Vulnerability details
Impact
Not sure what the use case of setPendingMintBalance() should be, but it can cause admins to drain the whole balance, nullify the user's funds or break the protocol. This can be caused on purpose, accidentally or through an attacker stealing MANAGER_ADMIN's private key.
Proof of Concept
If MANAGER_ADMIN calls
, it would cause userAddr to lose all the collateral deposited.
If MANAGER_ADMIN calls
, then managerAddr can mint any amount of cash and then redeem it to take all the collateral.
If MANAGER_ADMIN calls
, then the address can withdraw more tokens than he deposited and thus taking someone else's collateral out of the protocol (stealing someone else's funds).
Tools Used
Manual review
Recommended Mitigation Steps
Remove the function setPendingMintBalance