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

4 stars 0 forks source link

`_transferNFTs` doesn't revert for bad tokens #319

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

_transferNFTs checks if an item is ERC721 or ERC1155 by using IERC165(item.collection).supportsInterface(...).

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);
  }
}

The issue is that if an item is neither of that, the function doesn't revert. Basically the execution continues as if an item was transferred. If an user mistakenly used a wrong address, or if the NFT is something weird, someone will end up paying for buying nothing.

Proof of Concept

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

Recommended Mitigation Steps

Add a else with a revert statement.

nneverlander commented 2 years ago

Duplicate

HardlyDifficult commented 2 years ago

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