search

code-423n4 / 2023-10-party-findings

6 stars 4 forks source link

`PartyGovernance` and `PartyGovernanceNTF` do not comply with ERC165 #186

Closed c4-submissions closed 11 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernance.sol#L333-L339 https://github.com/code-423n4/2023-10-party/blob/b23c65d62a20921c709582b0b76b387f2bb9ebb5/contracts/party/PartyGovernanceNFT.sol#L113-L120

Vulnerability details

Impact

Protocols that check for compatibility using ERC-165 will receive incorrect value when calling supportsInterface() function.

The documentation straightforwardly states, that PartyGovernance and PartyGovernanceNTF should comply with ERC165

2023-10-party-protocol

The protocol is designed to comply with several Ethereum Improvement Proposals (EIPs). Following contracts should be in compliance:
PartyGovernance: Should comply with ERC4906
PartyGovernance: Should comply with ERC165
PartyGovernanceNFT: Should comply with ERC2981
PartyGovernanceNFT: Should comply with ERC721
PartyGovernanceNFT: Should comply with ERC165
ProposalExecutionEngine: Should comply with ERC1271
OffChainSignatureValidator: Should comply with ERC1271

According to this requirement, lack of ERC165 support has been evaluated as Medium risk.

Moreover, during the previous Code4rena contests, similar findings have been evaluated as Medium:

Proof of Concept

According to EIP-165:

source: https://eips.ethereum.org/EIPS/eip-165

A contract that is compliant with ERC-165 shall implement the following interface (referred as ERC165.sol):
(...)

The interface identifier for this interface is 0x01ffc9a7. You can calculate this by running bytes4(keccak256('supportsInterface(bytes4)')); or using the Selector contract above.

Therefore the implementing contract will have a supportsInterface function that returns:

    true when interfaceID is 0x01ffc9a7 (EIP165 interface)
    false when interfaceID is 0xffffffff
    true for any other interfaceID this contract implements
    false for any other interfaceID

Below functions, however, do not return true when interfaceID is 0x01ffc9a7, which implies they do not comply with ERC165.

File: PartyGovernance.sol

    function supportsInterface(bytes4 interfaceId) public pure virtual returns (bool) {
        return
            interfaceId == type(IERC721Receiver).interfaceId ||
            interfaceId == type(ERC1155TokenReceiverBase).interfaceId ||
            // ERC4906 interface ID
            interfaceId == 0x49064906;
    }

File: PartyGovernanceNFT.sol

function supportsInterface(
        bytes4 interfaceId
    ) public pure override(PartyGovernance, ERC721, IERC165) returns (bool) {
        return
            PartyGovernance.supportsInterface(interfaceId) ||
            ERC721.supportsInterface(interfaceId) ||
            interfaceId == type(IERC2981).interfaceId;
    }

Tools Used

Manual code review

Recommended Mitigation Steps

Return true when interfaceId == 0x01ffc9a7.

Assessed type

Other

ydspa commented 12 months ago

inheriting from here

 ERC721.supportsInterface(interfaceId) ||

Invalid

c4-pre-sort commented 12 months ago

ydspa marked the issue as insufficient quality report

c4-judge commented 11 months ago

gzeon-c4 marked the issue as unsatisfactory: Invalid