Oracle price can be manipulated within a certain time period

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/TradingExtension.sol#L163-L188
https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/utils/TradingLibrary.sol#L91-L122

Vulnerability details

Impact

The price and signature from an oracle is valid within a certain time period, determined by block timestamps. However, if multiple prices and signatures are released within this time period, the user can pick and choose which one to use, thus manipulating the price of positions.

Proof of Concept

TradingLibrary.verifyPrice verifies the signature of the price:

    function verifyPrice(
        uint256 _validSignatureTimer,
        uint256 _asset,
        bool _chainlinkEnabled,
        address _chainlinkFeed,
        PriceData calldata _priceData,
        bytes calldata _signature,
        mapping(address => bool) storage _isNode
    )
        external view
    {
        address _provider = (
            keccak256(abi.encode(_priceData))
        ).toEthSignedMessageHash().recover(_signature);
        require(_provider == _priceData.provider, "BadSig");
        require(_isNode[_provider], "!Node");
        require(_asset == _priceData.asset, "!Asset");
        require(!_priceData.isClosed, "Closed");
        require(block.timestamp >= _priceData.timestamp, "FutSig");
        require(block.timestamp <= _priceData.timestamp + _validSignatureTimer, "ExpSig");
        require(_priceData.price > 0, "NoPrice");
        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"
                );
            }
        }
    }

However, the signature is valid for _validSignatureTimer amount of time. Thus, if multiple signatures are released during a time period less than _validSignatureTimer, either of them are valid. The validSignatureTimer value is set by the owner in TradingExtension.sol. If this is too long, users are able to pick and choose what price to use.

Tools Used

n/a

Recommended Mitigation Steps

Carefully set validSignatureTimer and ensure it's the exact duration between prices/signatures being released from the oracle.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #108

[c4-judge]

GalloDaSballo marked the issue as satisfactory