OwnershipNFTs.sol does not declare `supportsInterface`

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/sol/OwnershipNFTs.sol#L13

Vulnerability details

Impact

NFTs cannot be identified as a valid ERC721 token during interactions with external integrations (e.g marketplace). It also doesn't comply with with EIP-721 standard, breaking composibility.

Proof of Concept

1. Context

The supportsInterface function in ERC721 tokens serves as the standard interface detection mechanism in Ethereum smart contracts. It allows other contracts or external entities to confirm that the ERC721 token contract is actually an NFT. This makes it easier for external contracts or integrations to interact with them in a standardized way.

2. Bug Location

OwnershipNFTs contract is missing the supportsInterface function. It is also missing in its inheritances and overall, in the entire codebase as evident from the search functionality.

pragma solidity 0.8.16;

import "./IERC721Metadata.sol";
import "./ISeawaterAMM.sol";

import "./IERC721TokenReceiver.sol";

/*
 * OwnershipNFTs is a simple interface for tracking ownership of
 * positions in the Seawater Stylus contract.
 */
contract OwnershipNFTs is IERC721Metadata {

<img width="1482" alt="supportsInterface" src="https://gist.github.com/user-attachments/assets/00443db0-d40b-4be7-aefb-343abbf93418">

3. Final Effect

The first effect is that EIP-721 standard is broken, as evident from the "MUST" keyword which, per RFC-2119 terminology, indicates a required trait in all NFT contracts.

Every ERC-721 compliant contract must implement the ERC721 and ERC165 interfaces

As a result of this, any external integration that depends on identifying ERC721 tokens as actual NFTs will fail since the function is absent in OwnershipNFTs.sol

Tools Used

Manual code review

Recommended Mitigation Steps

Recommend introducing the function, for example

+    function supportsInterface(bytes4 interfaceId) public view virtual returns (bool) {
+        return
+            interfaceId == 0x01ffc9a7 || // ERC165 Interface ID for ERC165
+            interfaceId == 0x80ac58cd || // ERC165 Interface ID for ERC721
+            interfaceId == 0x5b5e139f; // ERC165 Interface ID for ERC721Metadata
+    }

Assessed type

ERC721

[c4-judge]

alex-ppg changed the severity to QA (Quality Assurance)

[c4-judge]

alex-ppg marked the issue as grade-c

[c4-judge]

This previously downgraded issue has been upgraded by alex-ppg

[c4-judge]

alex-ppg changed the severity to QA (Quality Assurance)

[c4-judge]

alex-ppg marked the issue as grade-b

[alex-ppg]

Hey @ZanyBonzy, thank you for your PJQA feedback! As the Sponsor has outlined, there is no indication that the contract needs to be fully compliant with the EIP-721 specification. Claiming that a particular contract should implement a particular EIP when there is no explicit indication it should do so would open a can of worms for all C4 contests and would permit a lot of speculative findings to be submitted as valid which is something I do not condone.