Tigris Trade contest - gzeon's results

Lines of code

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

Vulnerability details

Impact

The value of blockDelay is unbounded. With the widespread use of unchecked throughout the codebase this can cause unexpected behavior. For example, it might overflow here:

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

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

and cause the delay not working as expected.

Proof of Concept

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

    function setBlockDelay(
        uint _blockDelay
    )
        external
        onlyOwner
    {
        blockDelay = _blockDelay;
    }

Recommended Mitigation Steps

Bound the max number of block delay, or remove the unchecked.

Lines of code

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

Vulnerability details

Impact

Chainlink latestAnswer() is deprecated. It might return stale data or incomplete round answer.

Proof of Concept

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

            int256 assetChainlinkPriceInt = IPrice(_chainlinkFeed).latestAnswer();

Recommended Mitigation Steps

Check for stale price and round completeness using latestRoundData()