code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

In case of _collateral in CashManager, if the collateral has over 18 decimals, decimalMultiplier will round the equation

Proof of Concept

In line

decimalsMultiplier =
      10 **
        (IERC20Metadata(_cash).decimals() -
          IERC20Metadata(_collateral).decimals());
  }

the protocol presumes that the difference between cash and collateral decimals will always be non-negative. In case this happens, the decimalsMultiplier will be rounded to 0, meaning in claimMint()

uint256 cashOwed = _getMintAmountForEpoch(...

will be 0, since

  function _getMintAmountForEpoch(
    uint256 collateralAmountIn,
    uint256 epoch
  ) private view returns (uint256 cashAmountOut) {
    uint256 amountE24 = _scaleUp(collateralAmountIn) * 1e6;
    cashAmountOut = amountE24 / epochToExchangeRate[epoch];
  }

will return 0, because _scaleup is 0.

This implies that the user will always get 0 cash no matter how much collateral he deposited.

Tools Used

Manual review

Recommended Mitigation Steps

If protocol only allows tokens with up to 18 decimals, there should be a condition checking that:

require(IERC20Metadata(_collateral).decimals()) <= 18, Error_message);

If not, implement _scaleUp in such a fashion:

 function _scaleUp(uint256 amount) private view returns (uint256) {
  if(IERC20Metadata(_collateral).decimals()) <= 18) {
    return amount * 10 **
        (IERC20Metadata(_cash).decimals() -
          IERC20Metadata(_collateral).decimals());
  } else {
  return amount / 10 **
        (IERC20Metadata(_collateral).decimals() - 
          IERC20Metadata(_cash).decimals());
  } 
}

c4-judge commented 1 year ago

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