Description

All of the oracles implement the latestRoundData function, which is defined in the AggregatorV3Interface:

function latestRoundData()
    external
    view
    returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound);

This interface is set by Chainlink, whose documentation↗ specifies that the startedAt and updatedAt values are timestamps.

However, in the Api3Aggregator function, the returned updatedAt value is a duration:

function latestRoundData()
    public
    view
    override
    returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)
{
    (int224 value, uint32 timestamp) = originPriceFeed.read();
    uint256 updatedAt_ = uint256(timestamp);
    int256 scaledTokenPrice = int256(int224(uint224(value)));
    return (uint80(1), scaledTokenPrice, updatedAt_, block.timestamp - updatedAt_, uint80(0));
}

Note that in the above code, the local variable updatedAt_ is a timestamp that is copied into the startedAt return value, and the updatedAt return value is calculated as block.timestamp - updatedAt_.

On the other hand, in the Api3LinkedAggregator contract, the updatedAt field is assigned a timestamp:

function latestRoundData()
    public
    view
    override
    returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)
{
    (int224 value, uint32 exchangeRateTimestamp) = exchangeRateFeed.read();

    // [...]

    return (
        tokenRoundId,
        finalPrice,
        uint256(exchangeRateTimestamp),
        uint256(exchangeRateTimestamp),
        tokenAnsweredInRound
    );
}

And, in the CompositeOracle contract, the updatedAt field is used as if it is a timestamp:

(, int256 answer,, uint256 updatedAt,) = abi.decode(data, (uint80, int256, uint256, uint256, uint80));
if (answer <= 0 || (block.timestamp - updatedAt) >= freshCheck) { // skip iteration if price is outdated.
    continue;
}

So in contrast with the second and third examples, the first example incorrectly assigns that return field.

Impact

There is currently no impact, because in the audited, frozen version of these contracts, the Api3Aggregator contract is unused, and the only oracle in use is the CompositeOracle, with underlying mock Chainlink oracles.

However, this issue represents a significant footgun for future oracle setups. For example, if a CompositeOracle is deployed whose aggregators include an Api3Aggregator, then the returned updatedAt value will be block.timestamp - updatedAt_, which would cause the comparison (block.timestamp - updatedAt) >= freshCheck to check updatedAt_ >= freshCheck. This is a comparison between a timestamp and a duration, so the duration will be interpreted as a timestamp shortly after January 1st, 1970. Thus, it will erroneously skip the current iteration and cause the protocol to retrieve suboptimal price data.

Recommendations

Fix the latestRoundData() function in Api3Aggregator to return the updatedAt value as a timestamp.

Remediation

This issue has been acknowledged by Takara Lend, and a fix was implemented in commit a5677e7b↗.