Unsafe downcast

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/modules/PRICE.sol#L75-L97

Vulnerability details

Impact

It's possible to generate silent overflows when downcasting. E.g. if the value if bigger than the type being cast, it will overflow starting from zero.

Proof of concept

For the Price.sol constructor, if observationFrequency_ is a small value and movingAverageDuration is a large value (bigger than 2**32 - 1). It's possible that that downcasting to uint32 will generate a slient overflow and cause the need for redeployments.

File: src/modules/PRICE.sol
constructor(
    Kernel kernel_,
    AggregatorV2V3Interface ohmEthPriceFeed_,
    AggregatorV2V3Interface reserveEthPriceFeed_,
    uint48 observationFrequency_,
    uint48 movingAverageDuration_
) Module(kernel_) {
    /// @dev Moving Average Duration should be divisible by Observation Frequency to get a whole number of observations
    if (movingAverageDuration_ == 0 || movingAverageDuration_ % observationFrequency_ != 0)
        revert Price_InvalidParams();

    // Set price feeds, decimals, and scale factor
    _ohmEthPriceFeed = ohmEthPriceFeed_;
    uint8 ohmEthDecimals = _ohmEthPriceFeed.decimals();

    _reserveEthPriceFeed = reserveEthPriceFeed_;
    uint8 reserveEthDecimals = _reserveEthPriceFeed.decimals();

    uint256 exponent = decimals + reserveEthDecimals - ohmEthDecimals;
    if (exponent > 38) revert Price_InvalidParams();
    _scaleFactor = 10**exponent;

    // Set parameters and calculate number of observations
    observationFrequency = observationFrequency_;
    movingAverageDuration = movingAverageDuration_;

    numObservations = uint32(movingAverageDuration_ / observationFrequency_);

Recommended Mitigation Steps

Consider using a safe cast library, or creating a private/internal utility functtion to revert if the cast generates an overflow.

function _safeCastUint32(uint256 n) internal pure returns (uint32) {
  require(n <= type(uint32).max);
  return int128(n);
}

[Oighty]

While this is technically correct, it will not practically occur. A Moving Average Duration of 2**32 seconds = 136.19... years.

[0xean]

closing as invalid w/ reasonable input values.