Connext Amarok contest - bardamu's results

Data returned from ChainLink aggregator is unchecked and could be stale/outdated

Lines of code

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

Vulnerability details

Impact

Price data returned by latestRoundData could be stale. This would create a situation where any contract consuming price information from Connext's oracle module would rely on an incorrect asset price.

Proof of Concept

As can be seen in function getPriceFromChainlink the answer from aggregator.latestRoundData() is assumed to be valid if it is non-zero and returned to the caller in getTokenPrice. However, aggregator data could be stale and therefore be outdated, which would propagate an incorrect price.

https://github.com/code-423n4/2022-06-connext/blob/main/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L122-L140

Tools Used

vim

Recommended Mitigation Steps

The caller should always check that latestRoundData is returning sane and up to date values.

Properties updatedAt and answeredInRound are of particular interest here. For example, answeredInRound could be checked against the current roundId to verify price information has been updated within a certain threshold.

function latestRoundData()
  public
  override
  view
  checkAccess()
  returns (
    uint80 roundId,
    int256 answer,
    uint256 startedAt,
    uint256 updatedAt,
    uint80 answeredInRound
  )
{
  return super.latestRoundData();
}