[SEE-04M] Insecure Order / Offer Key Generation

SEE-04M: Insecure Order / Offer Key Generation

Type	Severity	Location
Logical Fault		SalvorExchange.sol:L206, L229, L265, L286

Description:

The SalvorExchange contract will generate unique keys per order / offer to be able to track their fulfilment state, however, the way these keys are generated are prone to collisions and generally insecure.

Impact:

It is presently possible to hijack the fulfilment of an offer or order trivially by using the same NFT contract and salt even though the token ID of the hijacked order may not be held by the user.

Example:

/**
 * @notice Executes a purchase for a specific order within a batch order.
 * @param batchOrder The batch order containing the specific order to be executed.
 * @param signature The signature corresponding to the batch order.
 * @param position The position of the specific order within the batch order.
 */
function buy(LibOrder.BatchOrder memory batchOrder, bytes memory signature, uint256 position) internal {
    address seller = _validate(batchOrder, signature);
    address buyer = msg.sender;
    require(buyer != seller, "signer cannot redeem own coupon");

    LibOrder.Order memory order = batchOrder.orders[position];
    require(order.price > 0, "non existent order");
    require((block.timestamp - order.startedAt) < order.duration, "order has expired");

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

    emit Redeem(order.nftContractAddress, order.tokenId, order.salt, order.price);

    IAssetManager(assetManager).payMP(buyer, seller, order.nftContractAddress, order.tokenId, order.price);
}

Recommendation:

We advise the code to utilize the full order / offer hashes as keys instead of the constructed ones via LibOrder::hashKey and LibOrder::hashOfferKey, ensuring that a single adjustment in the order / offer data will lead to an entirely different fulfilment entry being queried.

salvorio / salvor-contracts