Incorrect implementation of `FarmFighter.sol#supportsInterface()` function (the `FighterFarm` contract don’t respect `ERC721Enumerable`)

[c4-bot-2]

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/main/src/FighterFarm.sol#L406-L417

Vulnerability details

Impact

The FighterFarm.sol contract inherit two contracts: ERC721 and ERC721Enumerable. So the FighterFarm.sol contract advertises support for the EIP-721 standard. In the EIP-721 standard writes the following:

supportInterface() function: Query if a contract implements an interface and return true if the contract implements interfaceID.

However the FighterFarm.sol#supportsInterface() function only call the supportInterface() of his base ERC721.sol contract without detecting the ERC721Enumerable.sol contract.

    /// @notice Returns whether a given interface is supported by this contract.
    /// @dev Calls ERC721.supportsInterface.
    /// @param _interfaceId The interface ID.
    /// @return Bool whether the interface is supported by this contract.
    function supportsInterface(bytes4 _interfaceId)
        public
        view
        override(ERC721, ERC721Enumerable)
        returns (bool)
    {
        return super.supportsInterface(_interfaceId);
    }

The incorrect implementation of the FighterFarm.sol#supportsInterface() function leads to a failure in accurately detecting support for the ERC721Enumerable interface. This oversight means that the contract not fully comply with the EIP721 standard, affecting interoperability with other contracts and protocols that rely on this standard for enumeration functionality. It impact the contract's ability to be listed or correctly interacted with on marketplaces, wallets, and other dApps that utilize interface detection to enable features or ensure compatibility.

Proof of Concept

• Code Reference: FighterFarm.sol#supportsInterface()
• EIP-721 Documentation: EIP-721

The supportsInterface() function in FighterFarm.sol incorrectly delegates the interface support check solely to the base ERC721 contract's implementation, neglecting to account for the ERC721Enumerable interface. This results in the contract not recognizing ERC721Enumerable-specific interface IDs as supported, even though FighterFarm extends ERC721Enumerable.

    /// @notice Returns whether a given interface is supported by this contract.
    /// @dev Calls ERC721.supportsInterface.
    /// @param _interfaceId The interface ID.
    /// @return Bool whether the interface is supported by this contract.
    function supportsInterface(bytes4 _interfaceId)
        public
        view
        override(ERC721, ERC721Enumerable)
        returns (bool)
    {
        return super.supportsInterface(_interfaceId);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

FighterFarm.sol#supportsInterface() function should be modified to explicitly check for ERC721Enumerable interfaceID alongside the base ERC721 checks.

    /// @notice Returns whether a given interface is supported by this contract.
    /// @dev Calls ERC721.supportsInterface.
    /// @param _interfaceId The interface ID.
    /// @return Bool whether the interface is supported by this contract.
    function supportsInterface(bytes4 _interfaceId)
        public
        view
        override(ERC721, ERC721Enumerable)
        returns (bool)
    {
        return interfaceId == type(ERC721Enumerable).interfaceId || super.supportsInterface(_interfaceId);
    }

This implementation ensures that the supportsInterface() function correctly identifies support for both the ERC721 and ERC721Enumerable interfaces.

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as duplicate of #118

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-judge]

HickupHH3 changed the severity to QA (Quality Assurance)

[radeveth]

Hey, @HickupHH3!

This is an EIP compatible problem, and in many previous code4rena contests this type of issue was marked as a valid medium difficulty problem.

The supportInterface() function is supposed to tell others if our contract supports certain standards like ERC721 and ERC721Enumerable. However, it turns out we've only been checking for ERC721 support and not including ERC721Enumerable in our checks.

This oversight means the contract is not fully compliant with the EIP-721 standard, which could hinder the contract's interaction with other apps and services that rely on these checks to work correctly. For example, marketplaces or wallets might not recognize all the features our contract offers, which is not ideal.

To fix this, the protocol need to update the supportsInterface() function to explicitly check for the ERC721Enumerable interface ID as well. This change will ensure that the FarmFighter.sol contract accurately reports support for both ERC721 and ERC721Enumerable standards, making it fully compatible with other parts of the Ethereum ecosystem that rely on this functionality.

[HickupHH3]

ref for ERC165: https://github.com/code-423n4/org/issues/130