Listings.sol

The Listings contract allows for an NFT that is held in the Locker to be listed above the floor price, with the lister prepaying a tax based on the price and duration of their listing.

Please also take note of the issues reported in NFTXHookCore.sol and NFTXZap.sol, as the permissionless nature of fillListings, cancelListings, modifyListings and relist contributes to both.

[MED] relist across two Listings contracts will lock the NFT in the Locker

The Locker supports up to 32 registered Listings contracts and Locker.isListing checks each of them before allowing an NFT to leave. However, each Listings contract only looks at its own storage when determining if a token is a floor item, and relist does not make any call to the Locker that would trigger the isListing check.

If two Listings contracts are registered, then anyone can call relist on the second contract for a token that already has an open listing on the first. The second contract will see no listing, price the token at FLOOR_VALUE, pay the original lister nothing and write a new listing. From this point both contracts hold a listing for the same token.

When either contract subsequently tries to cancel or fill the listing, it will delete its own listing and call Locker.withdrawToken, which will revert with TokenIsListing as the other contract still reports an owner:

function isListing(address _collection, uint _tokenId) public view returns (bool) {
    uint n = _listings.length();
    for (uint i; i < n; ++i) {
        if (IListings(_listings.at(i)).listings(_collection, _tokenId).owner != address(0)) {
            return true;
        }
    }
    return false;
}

Calls to redeem, swap and swapBatch will revert for the same reason, and closeStaleListing cannot be used as the Locker still holds the token. The NFT is frozen, the lister loses the premium that they listed for and the listingCount of both contracts is stuck above zero, which in turn blocks CollectionShutdown.execute for that collection. In our tests this cost the caller as little as 0.0015 of a collection token per NFT and could be repeated against every open listing.

The only way to recover from this is for the owner to call removeListings against one of the contracts, which removes the protection from every other open listing on that contract until it is added back.

As only one Listings contract is currently registered on each chain, this is not exploitable today. It will become exploitable as soon as a second is registered, which the documented migration process requires. This precondition is referenced in the deployment script and documentation, but it is not enforced onchain.

We would recommend that when relist finds no local listing for a token, it confirms this against the Locker before continuing:

// A floor item must be a floor item across all registered {Listings} contracts
if (oldListing.owner == address(0) && locker.isListing(_collection, _tokenId)) {
    revert TokenListedElsewhere();
}

It would also be beneficial for Locker.addListings to reject a second registration unless the new contract can show that it includes this validation.

[INFO] A fill in the same block will refund the full listing tax

When a listing is created, modified or relisted the tax window is restarted from the current block.timestamp. As _resolveListingTax refunds the unused tax pro rata, a fill that lands in the same block will refund the entire tax that was just paid. This is what allows the front-run described in NFTXZap.sol to be made for just the cost of gas, and should be considered alongside the fix for that issue.