Connext Amarok contest - Jujic's results

Chainlink's latestRoundData might return stale or incorrect results

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/4dd6149748b635f95460d4c3924c7e3fb6716967/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L125

Vulnerability details

Impact

On ConnextPriceOracle.sol, you are using latestRoundData, but there is no check if the return value indicates stale data.

Proof of Concept

function getPriceFromChainlink(address _tokenAddress) public view returns (uint256) {
    AggregatorV3Interface aggregator = aggregators[_tokenAddress];
    if (address(aggregator) != address(0)) {
      (, int256 answer, , , ) = aggregator.latestRoundData();

      // It's fine for price to be 0. We have two price feeds.
      if (answer == 0) {
        return 0;
      }

      // Extend the decimals to 1e18.
      uint256 retVal = uint256(answer);
      uint256 price = retVal.mul(10**(18 - uint256(aggregator.decimals())));

      return price;
    }

    return 0;
  }

Tools Used

This could lead to stale prices according to the Chainlink documentation: https://docs.chain.link/docs/historical-price-data/#historical-rounds https://docs.chain.link/docs/faq/#how-can-i-check-if-the-answer-to-a-round-is-being-carried-over-from-a-previous-round

Recommended Mitigation Steps

Add some checks in function getPriceFromChainlink()

...
 (uint80 roundID, int256 answer, , uint256 timestamp, uint80 answeredInRound ) = aggregator.latestRoundData();

        
        require(answeredInRound >= roundID, "Stale price");
        require(timestamp != 0,"Round not complete");
        
...