Using `exchangeRateStored()` leads to an understatement of the `cToken` value

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/periphery/oracles/CTokenOracle.sol#L38-L53

Vulnerability details

Impact

Current implementation of the _get() function means that the returned cToken value would be understated. Note that if this particular CToken isn't widely used, the exchangeRateStored() would easily be staled more significantly.

Proof of Concept

Take a look at CTokenOracle.sol#L38-L53

  /// @notice returns the price with 18 decimals without any state changes
  /// @dev some oracles require a state change to get the exact current price.
  ///      This is updated when calling other state changing functions that query the price
  /// @return _price the current price
  function peekValue() public view override returns (uint256 _price) {
    _price = _get();
  }

  /// @notice The current reported value of the oracle
  /// @return _value The current value
  //@audit-issue use exchangeRateCurrent() instead
  function _get() internal view returns (uint256 _value) {
    uint256 _exchangeRate = cToken.exchangeRateStored();
    uint256 _currentValue = anchoredViewUnderlying.peekValue();

    _value = (_currentValue * _exchangeRate) / div;
  }

As seen the _get() function is designed to calculate the current value of the oracle. This calculation is carried out by fetching the stored exchange rate from the cToken contract.

This implementation, however leads to an incorrect value due to the fact that it relies on the exchangeRateStored() function instead of the exchangeRateCurrent(), do note that both functions are readily available and the latter actually provides the up to date exchange rate since it first accrues interest and then returns price, implementation of both functions can be seen here

    /**
     * @notice Accrue interest then return the up-to-date exchange rate
     * @return Calculated exchange rate scaled by 1e18
     */
    function exchangeRateCurrent() override public nonReentrant returns (uint) {
        accrueInterest();
        return exchangeRateStored();
    }

    /**
     * @notice Calculates the exchange rate from the underlying to the CToken
     //@audit
     * @dev This function does not accrue interest before calculating the exchange rate
     * @return Calculated exchange rate scaled by 1e18
     */
    function exchangeRateStored() override public view returns (uint) {
        return exchangeRateStoredInternal();
    }

Since the oracle value is going to be used for critical operations the up to date exchange rate should be used instead so as not to constantly cause a mild misbalancing with each query that can accumulate over time.

Tool used

Manual Audit

Recommended Mitigation Steps

Consider replacing the instance of exchangeRateStored() with exchangeRateCurrent(). This would ensure that the most accurate and current exchange rate is used in the _get() function's calculations.

Assessed type

Context

[c4-pre-sort]

minhquanym marked the issue as duplicate of #19

[dmvt]

Impact not described

[c4-judge]

dmvt marked the issue as unsatisfactory: Insufficient quality