[SEE-06M] Insecure Order / Offer Cancellation Mechanism

[HKskn]

SEE-06M: Insecure Order / Offer Cancellation Mechanism

Type	Severity	Location
Logical Fault		SalvorExchange.sol:L206, L209, L283, L286

Description:

The SalvorExchange::cancelOffer and SalvorExchange::cancelOrder functions are insecure given that their signature validation methodology is insufficient in guarding against unwanted offer and order cancellations respectively.

Impact:

It is presently possible to cancel offers and more importantly orders arbitrarily as one can construct an order payload that would result in a conflicting orderKeyHash with another order, sign it themselves, and cancel it.

Example:

/**
 * @notice Cancels a specific order within a batch order.
 * @param batchOrder The batch order containing the specific order to be cancelled.
 * @param signature The signature corresponding to the batch order.
 * @param position The position of the specific order within the batch order.
 */
function cancelOrder(LibOrder.BatchOrder memory batchOrder, bytes memory signature, uint256 position) internal {
    LibOrder.Order memory order = batchOrder.orders[position];
    require(order.price > 0, "non existent order");
    require(msg.sender == _validate(batchOrder, signature), "only signer");

    bytes32 orderKeyHash = LibOrder.hashKey(order);
    require(!fills[orderKeyHash], "order has already redeemed or cancelled");
    fills[orderKeyHash] = true;

    emit Cancel(order.nftContractAddress, order.tokenId, order.salt);
}

Recommendation:

We advise the offerFills and fills mappings to utilize the full offer and order hashes respectively, and we additionally recommend the actual signer to be a member of the signed payload and validated as such.

[HKskn]

Fixed. We have modified the cancelOrder function by adding the cancelOrderInfo and cancelOrderSignature parameters to ensure that only an authorized wallet can call the cancelOrder function and validate that the caller is also the owner of the order. The reason for this change is to prevent active listed orders from becoming invalid.