Open code423n4 opened 1 year ago
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L195-L230
Minting will break if mintFee is set to zero
uint256 feesInCollateral = _getMintFees(collateralAmountIn); uint256 depositValueAfterFees = collateralAmountIn - feesInCollateral; _checkAndUpdateMintLimit(depositValueAfterFees); collateral.safeTransferFrom(msg.sender, feeRecipient, feesInCollateral);
CashManager#requestMint attempts to transfer fee to feeRecipient even if there is no fee to transfer (i.e. mintFee == 0). This will break minting for tokens that do not support zero value transfers if mintFee == 0.
CashManager#requestMint
feeRecipient
mintFee
Manual Review
Only transfer fees if there are fees to transfer:
uint256 feesInCollateral = _getMintFees(collateralAmountIn); uint256 depositValueAfterFees = collateralAmountIn - feesInCollateral; _checkAndUpdateMintLimit(depositValueAfterFees); - collateral.safeTransferFrom(msg.sender, feeRecipient, feesInCollateral); + if(feesInCollateral != 0) { + collateral.safeTransferFrom(msg.sender, feeRecipient, feesInCollateral); + }
trust1995 changed the severity to QA (Quality Assurance)
trust1995 marked the issue as grade-b
Lines of code
https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L195-L230
Vulnerability details
Impact
Minting will break if mintFee is set to zero
Proof of Concept
CashManager#requestMint
attempts to transfer fee tofeeRecipient
even if there is no fee to transfer (i.e.mintFee
== 0). This will break minting for tokens that do not support zero value transfers ifmintFee
== 0.Tools Used
Manual Review
Recommended Mitigation Steps
Only transfer fees if there are fees to transfer: