sherlock-audit / 2023-01-sentiment-judging

2 stars 0 forks source link

Madalad - Chainlink oracle data feed becoming blocked severely affects protocol's usability #11

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Madalad

medium

Chainlink oracle data feed becoming blocked severely affects protocol's usability

Summary

Access to ChainLink price feeds may be blocked at any point in time, which would cause the functions getPrice and getEthPrice from GLPOracle to always revert, resulting in a denial of service.

Vulnerability Detail

Currently, GLPOracle uses oracle data from ChainLink's ETH USDC price feed. As mentioned here, it is possible that ChainLink's "multisigs can immediately block access to price feeds at will". It is worth noting that this is not certain addresses being added to a blacklist, but rather the entire price feed becoming unusable by anyone. When this occurs, the call to ethUsdPriceFeed.latestRoundData() within getEthPrice will always cause a revert, meaning that getPrice will also guarantee a revert. This causes a denial of service and renders the oracle completely unusable, as well as any other external methods that rely on it, which may include borrowing and lending.

Impact

If GLPOracle is being used within the lending and borrowing processes, then denial of service in this manner may lead to users being unable to lend, borrow, repay or liquidate positions. In this case funds would essentially become frozen, and depending on the specific implementation, may even be permanently lost.

Code Snippet

https://github.com/sentimentxyz/oracle/blob/rage-jr/src/gmx/GLPOracle.sol#L48-L49

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);
}

Tool used

Manual Review

Recommendation

As described in the article linked above: "to prevent denial of service scenarios, it is recommended to query ChainLink price feeds using a defensive approach with Solidity’s try/catch structure. In this way, if the call to the price feed fails, the caller contract is still in control and can handle any errors safely and explicitly".

function getEthPrice() internal view returns (uint) {
    try ethUsdPriceFeed.latestRoundData() returns (
        uint80,
        int answer,
        uint256,
        uint256 updatedAt,
        uint80
    ) {
        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);
    } catch Error(string memory) {
        // handle error
    }
}