search

code-423n4 / 2022-06-infinity-findings

4 stars 0 forks source link

`_transferNFTs()` functions doesn't reverts if the transfer item doesn't supports interface for both ERC721 and ERC1155 #310

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-infinity/blob/main/contracts/core/InfinityExchange.sol#L1062-L1072

Vulnerability details

Impact

The _transferNFTs function use ERC165 to check if the item(nft) supports ERC721 interface or ERC1155 interface and execute transfer accordingly. But if it doesn't supports either, it just exits the function(no revert).

Proof of Concept

in InfinityExchange.sol#L1062-L1072

  /**
   * @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);
    }
  }

Tools Used

Manual Analysis

Recommended Mitigation Steps

Revert the transaction if the item doesn't supports any NFT specification

  /**
   * @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);
    } else {
      require(false, "The item is not an NFT");
    }
  }
nneverlander commented 2 years ago

Duplicate

HardlyDifficult commented 2 years ago

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