search

sherlock-audit / 2024-01-telcoin-judging

6 stars 5 forks source link

Jaraxxus - CouncilMember.sol does not comply with ERC721, breaking composability #188

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

Jaraxxus

medium

CouncilMember.sol does not comply with ERC721, breaking composability

Summary

The CouncilMember NFT does not comply with ERC721.

Vulnerability Detail

CouncilMember.sol implements supportsInterface as shown below.


    function supportsInterface(
        bytes4 interfaceId
    )
        public
        pure
        override(
            AccessControlEnumerableUpgradeable,
            ERC721EnumerableUpgradeable
        )
        returns (bool)
    {
        return
            interfaceId ==
            type(AccessControlEnumerableUpgradeable).interfaceId ||
            interfaceId == type(ERC721EnumerableUpgradeable).interfaceId;
    }

The function only supports ERC721 extension , and does not comply with ERC721 itself. From EIP721:

"Every ERC-721 compliant contract must implement the ERC721 and ERC165 interfaces (subject to “caveats” below):"

Impact

Any contract that will make sure it is dealing with an ERC721 compliant NFT will not interoperate with Council Member tokens.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin-cryptostaker2/blob/475e5c92315d0b6cc1aa185a856fb8c24d993040/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L146-L161

Tool used

Manual Review

Recommendation

Recommend adding interfaceId == type(IERC721).interfaceId to comply with ERC721 as well.

Duplicate of #108

sherlock-admin2 commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { This is also invalid and same as issue 108}