A sophisticated user can pick a price from the past _validSignatureTimer as _priceData to open positions for profit

[code423n4]

Lines of code

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

Vulnerability details

Impact

initiateMarketOrder() can be called with a signed off-chain price data from the past _validSignatureTimer and that price will be taken as the open price of the position.

Because the difference of the current price to the price from _validSignatureTimer ago is known to the caller, it will be possible for the user to create a long position at a price 2% lower than the current market price, or a short position at a price 2% higher than the current market price.

If the _validSignatureTimer is long enough, combining with leverage, the attacker will be able to disrupt the market by draining all the profits from the other regular users.

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

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

Recommended Mitigation Steps

Signed _priceData should only be allowed for SL/TP and match limit orders. For those orders, only a priceData with a timestamp after the created timestamp of the order can be used.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #108

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)

[c4-judge]

GalloDaSballo marked the issue as satisfactory