Description

Within the _matchIncomingAsk function, the code branch for removing expired orders does not update the bestBid variable using ds.getBestBid() before executing the continue statement. Consequently, bestBid can become stale, potentially referring to the price of a removed order in the next loop iteration.

function _matchIncomingAsk(PerpBook storage ds, Order memory incomingOrder, bool amountIsBase)
    internal
    returns (uint256 totalQuoteTokenReceived, uint256 totalBaseTokenSent, uint256 totalTakerFeeInQuote)
{
    uint256 bestBid = ds.getBestBid();

    while (bestBid >= incomingOrder.price && incomingOrder.amount > 0) {
        // [...]

        if (bestBidOrder.isExpired()) {
            _removeUnfillableOrder(ds, bestBidOrder);
            continue;
        }
        // [...]
        bestBid = ds.getBestBid();
    }
    // [...]
}

Similarly, in the _matchIncomingBid function, the code does not update the bestAsk variable using ds.getBestAsk() after removing an expired order and before the continue statement.

function _matchIncomingBid(PerpBook storage ds, Order memory incomingOrder, bool amountIsBase)
    internal
    returns (MatchResult memory result)
{
    uint256 bestAsk = ds.getBestAsk();

    while (bestAsk <= incomingOrder.price && incomingOrder.amount > 0) {
        // [...]
        if (bestAskOrder.isExpired()) {
            _removeUnfillableOrder(ds, bestAskOrder);
            continue;
        }
        // [...]
        bestAsk = ds.getBestAsk();
    }
    // [...]
}

Impact

If, after removing an expired order, no orders remain at that price, the subsequent _matchIncomingOrder call would use an empty order. This will cause the function to fail when it tries to remove this empty order at the end of _matchIncomingOrder.

As a result, a filler’s transaction will fail if there is an expired order at the end of the limit order array for a matched price.

Recommendations

We recommend updating the bestBid variable (in _matchIncomingAsk) and the bestAsk variable (in _matchIncomingBid) immediately after an expired order is removed and before the continue statement is executed.

Remediation

This issue has been acknowledged by Liquid Labs, Inc., and a fix was implemented in commit 2d5e753c↗.