Tigris Trade contest - fs0c's results

Lines of code

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

Vulnerability details

Impact

The latestAnswer might return 0 which might pass the price checks. verifyPrice is called in getVerifiedPrice in TradingExtension.sol which is called by most of the functions user interaction takes place. If a malicious user places in wrong price and the latestAnswer fails/returns 0, the price check would fail and the user would be able to place a position with the price of their choice.

POC

According to Chainlink’s documentation (latestAnswer Deprecated API Reference), the latestAnswerfunction is deprecated. This function does not throw an error if no answer has been reached, but instead returns 0, causing an incorrect price to be fed to the TradingLibrary.sol,

The verifyPrice function in TradingLibrary.sol is using the deprecated API and there is no checks if the value returned from latestAnswer is 0. Furthermore it just skips and passes all the require statements if the price fetched from latestAnswer is 0.

function verifyPrice(
        uint256 _validSignatureTimer,
        uint256 _asset,
        bool _chainlinkEnabled,
        address _chainlinkFeed,
        PriceData calldata _priceData,
        bytes calldata _signature,
        mapping(address => bool) storage _isNode
    )
        external view
    {
        // ... code snipped to show the bug.
        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"
                );
            }
        }
    }
}

Recommendation

Use the latestRoundData function to get the price instead. Add checks on the return data with proper revert messages if the price is stale or the round is incomplete.