sherlock-audit / 2023-01-sentiment-judging

2 stars 0 forks source link

ctf_sec - Lack of sufficient validation for chainlink price feed #29

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

ctf_sec

medium

Lack of sufficient validation for chainlink price feed

Summary

Lack of sufficient validation for chainlink price feed

Vulnerability Detail

In the current implementation of the chainlink price feed integration:

the round id is not validated.

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

According to the chainlink documentation:

https://docs.chain.link/data-feeds/price-feeds/historical-data/

roundId and answeredInRound are also returned. "You can check answeredInRound against the current roundId. If answeredInRound is less than roundId, the answer is being carried over. If answeredInRound is equal to roundId, then the answer is fresh."

Impact

WIthout validating the round id, stale or invalid price can be used.

In the function getPrice

/// @inheritdoc IOracle
function getPrice(address) external view returns (uint) {
    return manager.getPrice(false) / (getEthPrice() * 1e4);
}

Code Snippet

https://github.com/sherlock-audit/2023-01-sentiment/blob/main/oracle/src/gmx/GLPOracle.sol#L48

https://github.com/sherlock-audit/2023-01-sentiment/blob/main/oracle/src/gmx/GLPOracle.sol#L49

Tool used

Manual Review

Recommendation

We recommend the protocol validate the round id

(uint80 roundId, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = ethUsdPriceFeed.latestRoundData();

require(answeredInRound >= roundId, "answer is stale");

// rest of the validation.

Duplicate of #31