Tigris Trade contest - SmartSek's results

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/496e1974ee3838be8759e7b4096dbee1b8795593/contracts/Trading.sol#L893-L905

Vulnerability details

Impact

blockDelay could be accidentally set to a large value and lead to positions never closing. and Profitable opening and closing can be executed in the same transaction with two different prices in the "valid signature pool" until setBlockDelay() is called.

Proof of Concept

No boundaries when setting blockDelay. https://github.com/code-423n4/2022-12-tigris/blob/496e1974ee3838be8759e7b4096dbee1b8795593/contracts/Trading.sol#L893-L905

    /**
     * @dev Sets block delay between opening and closing
     * @notice In blocks not seconds
     * @param _blockDelay delay amount
     */
    function setBlockDelay(
        uint _blockDelay
    )
        external
        onlyOwner
    {
        blockDelay = _blockDelay;
    }

https://github.com/code-423n4/2022-12-tigris/blob/496e1974ee3838be8759e7b4096dbee1b8795593/contracts/Trading.sol#L857-L868

    function _checkDelay(uint _id, bool _type) internal {
        unchecked {
            Delay memory _delay = blockDelayPassed[_id];
            if (_delay.actionType == _type) {
                blockDelayPassed[_id].delay = block.number + blockDelay;
            } else {
                if (block.number < _delay.delay) revert("0"); //Wait
                blockDelayPassed[_id].delay = block.number + blockDelay;
                blockDelayPassed[_id].actionType = _type;
            }
        }
    }

Recommended Mitigation Steps

Implement boundaries so there is no risk of accidentally setting it to a value too large or early users having a chance to execute opening and closing trades in the same transaction.