code-423n4/2022-12-tigris-findings

TradingLibrary#verifyPrice doesn't check if data is fresh which can lead to costly downtime

code423n4 opened this issue · 2 comments

code423n4 commented

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/588c84b7bb354d20cbca6034544c4faa46e6a80e/contracts/utils/TradingLibrary.sol#L113

Vulnerability details

Impact

verifyPrice may check against stale data causing valid transactions to revert

Proof of Concept

    if (_chainlinkEnabled && _chainlinkFeed != address(0)) {
        int256 assetChainlinkPriceInt = IPrice(_chainlinkFeed).latestAnswer();
        if (assetChainlinkPriceInt != 0) {
            uint256 assetChainlinkPrice = uint256(assetChainlinkPriceInt) * 10**(18 - IPrice(_chainlinkFeed).decimals());
            require(
                _priceData.price < assetChainlinkPrice+assetChainlinkPrice*2/100 &&
                _priceData.price > assetChainlinkPrice-assetChainlinkPrice*2/100, "!chainlinkPrice"
            );
        }
    }

Chainlink prices are used as a fail-safe in case malicious price data is being produced by a node. When pulling the price it never checks if the price is fresh. This can lead to legitimate transactions reverting if the Chainlink price is stale, causing the whole system to freeze until the oracle is back online.

Tools Used

Manual Review

Recommended Mitigation Steps

Downtime in a futures market can be extremely costly. Since the Chainlink oracle is just a fail-safe it should be bypassed if the data is stale

c4-judge commented

GalloDaSballo marked the issue as duplicate of #655

c4-judge commented

GalloDaSballo marked the issue as satisfactory