_transferNFTs can end up transferring nothing

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1056-L1073

Vulnerability details

Malicious maker can list an NFT that conforms to ERC-165, but reports that it's neither ERC721, nor ERC1155, i.e. both supportsInterface(0x80ac58cd) and supportsInterface(0xd9b67a26) are false. In all other regards it can be fully valid NFT, for example having well-know NFT, say Crypto punk, in a transparent custody, and having no other issues.

When a taker attempts to buy it, however, the InfinityExchange simply will not transfer it to him, while transferring his bid to the maker, who steals from the taker this way. As this specifics of the NFT isn't malicious per se, i.e. it doesn't steal simply by reporting the wrong state of interface support, it will not be flagged as or known to be faulty.

Proof of Concept

_transferNFTs performs a noop when NFT doesn't conform to both standards:

https://github.com/code-423n4/2022-06-infinity/blob/765376fa238bbccd8b1e2e12897c91098c7e5ac6/contracts/core/InfinityExchange.sol#L1056-L1073

  /**
   * @notice Transfer NFTs
   * @param from address of the sender
   * @param to address of the recipient
   * @param item item to transfer
   */
  function _transferNFTs(
    address from,
    address to,
    OrderTypes.OrderItem calldata item
  ) internal {
    if (IERC165(item.collection).supportsInterface(0x80ac58cd)) {
      _transferERC721s(from, to, item);
    } else if (IERC165(item.collection).supportsInterface(0xd9b67a26)) {
      _transferERC1155s(from, to, item);
    }
  }

However, at this point all other obligations are considered to be met and maker's bid for the NFT is transferred to taker, who can steal it this way by listing such an NFT that simply do not support both interfaces.

Recommended Mitigation Steps

Consider requiring conformity and performing unconditional transfer, so there can be no case of unmet obligations.

[nneverlander]

Fixed in https://github.com/infinitydotxyz/exchange-contracts-v2/commit/377c77f0888fea9ca1e087de701b5384a046f760

[nneverlander]

https://github.com/code-423n4/2022-06-infinity-findings/issues/43

[HardlyDifficult]

Dupe https://github.com/code-423n4/2022-06-infinity-findings/issues/87