# 29068 - \[SC - Medium] AaveOracle contract does not verify price stale...

Submitted on Mar 6th 2024 at 14:02:31 UTC by @Paludo0x for [Boost | ZeroLend](https://immunefi.com/bounty/zerolend-boost/)

Report ID: #29068

Report type: Smart Contract

Report severity: Medium

Target: <https://explorer.zksync.io/address/0x785765De3E9ac3D8eEb42B4724A7FEA8990142B8>

Impacts:

* Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield

## Description

## Brief/Intro

**AaveOracle** contract relies on Pyth oracles to retrieve assets price feeds and the actual implementation doesn't check whether prices are fresh or staled. Assets prices are retrieved by means of function `AaveOracle::getAssetPrice()`. For instance this function is called by `LiquidationLogic::_calculateAvailableCollateralToLiquidate` which *"calculates how much of a specific collateral can be liquidated, given a certain amount of debt asset"*. Therefore a wrong calculation can lead to liquidate q wrong amount of base assets.

## Vulnerability Details

This is the code snippet of function `AaveOracle::getAssetPrice()`.

```
  function getAssetPrice(address asset) public view override returns (uint256) {
    AggregatorInterface source = assetsSources[asset];

    if (asset == BASE_CURRENCY) {
      return BASE_CURRENCY_UNIT;
    } else if (address(source) == address(0)) {
      return _fallbackOracle.getAssetPrice(asset);
    } else {
      int256 price = source.latestAnswer();
      if (price > 0) {
        return uint256(price);
      } else {
        return _fallbackOracle.getAssetPrice(asset);
      }
    }
  }
```

The price oracle address is stored in mapping `assetsSources[asset]`. The price is retrieved by means of `int256 price = source.latestAnswer();` and only if `price = 0`the **\_fallbackOracle** is called. Therefore even if price is far from the actual market price it's deemed acceptable.

Let's take as an example **<https://explorer.zksync.io/address/0xf531672C92Ad4658c54B4fBE855029Df43c57390#contract>** which is the oracle for asset **0x4B9eb6c0b6ea15176BBF62841C6B2A8a398cb656** (Dai Stablecoin).

You can see that `PythAggregatorV3::latestAnswer()` calls `pyth.getPriceUnsafe(priceId);`

```
    function latestAnswer() public view virtual returns (int256) {
        PythStructs.Price memory price = pyth.getPriceUnsafe(priceId);
        return int256(price.price);
    }
```

`pyth` address correspond to 0xf087c864AEccFb6A2Bf1Af6A0382B0d0f6c5D834 which is a proxy contract. While the implementation can be found here: <https://explorer.zksync.io/address/0xb1b239054fa2e37da736e43102d175f21c5f7450#contract>

You can see that `getPriceUnsafe` just query the price feed without checking if it is up to date

```
function getPriceUnsafe(
    bytes32 id
) public view virtual override returns (PythStructs.Price memory price) {
    PythStructs.PriceFeed memory priceFeed = queryPriceFeed(id);
    return priceFeed.price;
}
```

## Impact Details

This are examples of where `AaveOracle::getAssetPrice()` is used:

* `LiquidationLogic::_calculateAvailableCollateralToLiquidate` which is called by `LiquidationLogic::executeLiquidationCall` which i turns is called by Pool::liquidationCall\`
* `GenericLogic::calculateUserAccountData` retrieves assets price to verify account health factor, and it's called in turn by `ValidationLogic::validateHealthFactor` or `ValidationLogic.validateBorrow`

That means that if price is staled, especially in a turbolent market condition, can lead to incorrect liquidations or worse, lack of liquidation, and health factor wrong calculation

## Fix suggestion

The advice is to implement a verification if the feed last update is within a thresold by means of `PythAggregatorV3::latestTimestamp()`

```
function latestTimestamp() public view returns (uint256) {
        PythStructs.Price memory price = pyth.getPriceUnsafe(priceId);
        return price.publishTime;
    }

```

## Proof of concept

The requirement of price staleness verification is suggested by Pyth protocol in NATSPEC of function `pyth.getPriceUnsafe(priceId);`

```
 /// @notice Returns the price of a price feed without any sanity checks.
    /// @dev This function returns the most recent price update in this contract without any recency checks.
    /// This function is unsafe as the returned price update may be arbitrarily far in the past.
    ///
    /// Users of this function should check the `publishTime` in the price to ensure that the returned price is
    /// sufficiently recent for their application. If you are considering using this function, it may be
    /// safer / easier to use either `getPrice` or `getPriceNoOlderThan`.
    /// @return price - please read the documentation of PythStructs.Price to understand how to use this safely.
```

Next the following sentence can be found at function docs: <https://docs.pyth.network/price-feeds/api-reference/evm/get-price-unsafe>

*"This function may return a price from arbitrarily far in the past. It is the caller's responsibility to check the returned publishTime to ensure that the update is recent enough for their use case."*


---

# Agent Instructions: Querying This Documentation

If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question.

Perform an HTTP GET request on the current page URL with the `ask` query parameter:

```
GET https://reports.immunefi.com/zerolend/29068-sc-medium-aaveoracle-contract-does-not-verify-price-stale....md?ask=<question>
```

The question should be specific, self-contained, and written in natural language.
The response will contain a direct answer to the question and relevant excerpts and sources from the documentation.

Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.
