sherlock-audit / 2024-04-titles-judging

6 stars 6 forks source link

ZdravkoHr. - `Edition.supportsInterface` is not EIP1155 compliant #287

Open sherlock-admin4 opened 3 months ago

sherlock-admin4 commented 3 months ago

ZdravkoHr.

medium

Edition.supportsInterface is not EIP1155 compliant

Summary

According to the ERC-1155 specification, the smart contracts that are implementing it MUST have a supportsInferface(bytes4) function that returns true for values 0xd9b67a26 and 0x0e89341c. The current implementation of Edition.sol will return false for both these values.

Vulnerability Detail

The contract inherits from ERC1155 and ERC2981.

contract Edition is IEdition, ERC1155, ERC2981, Initializable, OwnableRoles

The supportsInterface() function of Edition returns the result of executing super.supportsInterface()

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(IEdition, ERC1155, ERC2981)
        returns (bool)
    {
        return super.supportsInterface(interfaceId);
    }

Since both ERC1155 and ERC2981 implement that function and ERC2981 is the more derived contract of the two, Edition.supportsInterface() will end up executing only ERC2981.supportsInterface().

Impact

Medium. The contract is to be a strict implementation of ERC1155, but it does not implement the mandatory ERC1155.supportsInterface() function.

Code Snippet

PoC for Edition.t.sol

    function test_interface() public {
        assertFalse(edition.supportsInterface(bytes4(0xd9b67a26)));
        assertFalse(edition.supportsInterface(bytes4(0x0e89341c)));
    }

Tool used

Foundry

Recommendation

Instead of relying on super, return the union of ERC1155.supportsInterface(interfaceId) and ERC2981.supportsInterface(interfaceId).

    function supportsInterface(bytes4 interfaceId)
        public
        view
        override(IEdition, ERC1155, ERC2981)
        returns (bool)
    {
-       return super.supportsInterface(interfaceId);
+       return ERC1155.supportsInterface(interfaceId) || ERC2981.supportsInterface(interfaceId);
    }
sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/titlesnyc/wallflower-contract-v2/pull/1

sherlock-admin2 commented 1 month ago

The Lead Senior Watson signed off on the fix.