Lack of freshness check in GLPOracle.getEthPrice()

Summary

Oracle may return a stale price.

Vulnerability Detail

In GLPOracle.sol, latestRoundData() is used but there is no check if the return value indicates stale data.

    function getEthPrice() internal view returns (uint) {
        (, int answer,, uint updatedAt,) =
            ethUsdPriceFeed.latestRoundData();

        if (block.timestamp - updatedAt >= 86400)
            revert Errors.StalePrice(address(0), address(ethUsdPriceFeed));

        if (answer <= 0)
            revert Errors.NegativePrice(address(0), address(ethUsdPriceFeed));

        return uint(answer);
    }
}

Oracle could return 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

Impact

Oracle may return a stale price.

Code Snippet

https://github.com/sentimentxyz/oracle/pull/49/files#diff-8a14b9680f56e2aa29118d7fd07ed8dbdaf5f3f3d2be7c8e69fb4223b8659202R47-R57

https://github.com/sentimentxyz/oracle/blob/e940955eba38562d2e27059787294e458e28fe7e/src/gmx/GLPOracle.sol#L47-L59

Tool used

Manual Review

Recommendation

Check for latest roundID as well.

    function getEthPrice() internal view returns (uint) {
+        (uint80 roundID, int answer, , uint updatedAt, uint80 answeredInRound) =
            ethUsdPriceFeed.latestRoundData();
+        require(answeredInRound >= roundID, "Stale price");
        if (block.timestamp - updatedAt >= 86400)
            revert Errors.StalePrice(address(0), address(ethUsdPriceFeed));

        if (answer <= 0)
            revert Errors.NegativePrice(address(0), address(ethUsdPriceFeed));

        return uint(answer);
    }
}

Duplicate of #31

sherlock-audit / 2023-01-sentiment-judging

peanuts - Lack of freshness check in GLPOracle.getEthPrice() #21