Fix ERC165Checker detection (#5880)

Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: James Toussaint <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+### Bug fixes
+
+- `ERC165Checker`: Ensure the `supportsERC165` function returns false if the target reverts during the `supportsInterface(0xffffffff)` call. ([#5810](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5880))
 
 ### Breaking changes
 

contracts/mocks/ERC165/ERC165MaliciousData.sol

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.20;
 
-import {IERC165} from "../../utils/introspection/IERC165.sol";
+import {IERC165} from "../utils/introspection/IERC165.sol";
 
 /**
  * https://eips.ethereum.org/EIPS/eip-214#specification
@@ -36,7 +36,7 @@ contract SupportsInterfaceWithLookupMock is IERC165 {
     /**
      * @dev Implement supportsInterface(bytes4) using a lookup table.
      */
-    function supportsInterface(bytes4 interfaceId) public view override returns (bool) {
+    function supportsInterface(bytes4 interfaceId) public view virtual override returns (bool) {
         return _supportedInterfaces[interfaceId];
     }
 
@@ -56,3 +56,45 @@ contract ERC165InterfacesSupported is SupportsInterfaceWithLookupMock {
         }
     }
 }
+
+// Similar to ERC165InterfacesSupported, but revert (without reason) when an interface is not supported
+contract ERC165RevertInvalid is SupportsInterfaceWithLookupMock {
+    constructor(bytes4[] memory interfaceIds) {
+        for (uint256 i = 0; i < interfaceIds.length; i++) {
+            _registerInterface(interfaceIds[i]);
+        }
+    }
+
+    function supportsInterface(bytes4 interfaceId) public view override returns (bool) {
+        require(super.supportsInterface(interfaceId));
+        return true;
+    }
+}
+
+contract ERC165MaliciousData {
+    function supportsInterface(bytes4) public pure returns (bool) {
+        assembly {
+            mstore(0, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
+            return(0, 32)
+        }
+    }
+}
+
+contract ERC165MissingData {
+    function supportsInterface(bytes4 interfaceId) public view {} // missing return
+}
+
+contract ERC165NotSupported {}
+
+contract ERC165ReturnBombMock is IERC165 {
+    function supportsInterface(bytes4 interfaceId) public pure override returns (bool) {
+        if (interfaceId == type(IERC165).interfaceId) {
+            assembly {
+                mstore(0, 1)
+            }
+        }
+        assembly {
+            return(0, 101500)
+        }
+    }
+}

contracts/mocks/ERC165/ERC165MissingData.sol

@@ -22,9 +22,12 @@ library ERC165Checker {
     function supportsERC165(address account) internal view returns (bool) {
         // Any contract that implements ERC-165 must explicitly indicate support of
         // InterfaceId_ERC165 and explicitly indicate non-support of InterfaceId_Invalid
-        return
-            supportsERC165InterfaceUnchecked(account, type(IERC165).interfaceId) &&
-            !supportsERC165InterfaceUnchecked(account, INTERFACE_ID_INVALID);
+        if (supportsERC165InterfaceUnchecked(account, type(IERC165).interfaceId)) {
+            (bool success, bool supported) = _trySupportsInterface(account, INTERFACE_ID_INVALID);
+            return success && !supported;
+        } else {
+            return false;
+        }
     }
 
     /**
@@ -106,19 +109,34 @@ library ERC165Checker {
      * Interface identification is specified in ERC-165.
      */
     function supportsERC165InterfaceUnchecked(address account, bytes4 interfaceId) internal view returns (bool) {
-        // prepare call
-        bytes memory encodedParams = abi.encodeCall(IERC165.supportsInterface, (interfaceId));
+        (bool success, bool supported) = _trySupportsInterface(account, interfaceId);
+        return success && supported;
+    }
+
+    /**
+     * @dev Attempts to call `supportsInterface` on a contract and returns both the call
+     * success status and the interface support result.
+     *
+     * This function performs a low-level static call to the contract's `supportsInterface`
+     * function. It returns:
+     *
+     * * `success`: true if the call didn't revert, false if it did
+     * * `supported`: true if the call succeeded AND returned data indicating the interface is supported
+     */
+    function _trySupportsInterface(
+        address account,
+        bytes4 interfaceId
+    ) private view returns (bool success, bool supported) {
+        bytes4 selector = IERC165.supportsInterface.selector;
 
-        // perform static call
-        bool success;
-        uint256 returnSize;
-        uint256 returnValue;
         assembly ("memory-safe") {
-            success := staticcall(30000, account, add(encodedParams, 0x20), mload(encodedParams), 0x00, 0x20)
-            returnSize := returndatasize()
-            returnValue := mload(0x00)
+            mstore(0x00, selector)
+            mstore(0x04, interfaceId)
+            success := staticcall(30000, account, 0x00, 0x24, 0x00, 0x20)
+            supported := and(
+                gt(returndatasize(), 0x1F), // we have at least 32 bytes of returndata
+                iszero(iszero(mload(0x00))) // the first 32 bytes of returndata are non-zero
+            )
         }
-
-        return success && returnSize >= 0x20 && returnValue > 0;
     }
 }