CreateOfferer is not SIP-compliant, which can cause integration issues with third parties

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/main/src/CreateOfferer.sol#L241-L243

Vulnerability details

Impact

CreateOfferer is not SIP-compliant and this might cause issues with integrations that expect it. CreateOfferer can't be indexed by Seaport due to the lack of SIP-5 integration.

Proof of Concept

SIPs are the equivalent of EIPs in the Seaport ecosystem.

CreateOfferer is meant to be used to provide gasless Delegate token listings on Seaport: it implements ContractOffererInterface, which expects the Seaport app to integrate getSeaportMetadata:

/// @notice Implementation of seaport contract offerer getSeaportMetadata
function getSeaportMetadata() external pure returns (string memory, Schema[] memory) {
    return ("Delegate Market Contract Offerer", new Schema[](0));
}

https://github.com/code-423n4/2023-09-delegate/blob/main/src/CreateOfferer.sol#L241-L243

This implementation is not SIP compliant, as it lacks the following:

1. The contract MUST emit the SeaportCompatibleContractDeployed event when deployed
2. The contract MUST adhere to EIP-165 and provide a supportsInterface function that returns true for the interface of getSeaportMetadata. The contract MUST also return true for supportsInterface of the Seaport interface being implemented.
3. The contract MUST return a single schema ID for each additional SIP they support; they MUST NOT return any duplicate schema IDs and SHOULD return schema IDs in order from lowest to highest.

Tools Used

Manual review

Recommended Mitigation Steps

Consider integrating the missing aspects described at the end of this POC to have a fully SIP-5-compliant contract.

Assessed type

Other

[c4-sponsor]

0xfoobar marked the issue as disagree with severity

[0xfoobar]

This is QA issue as there is ~0 SIP infra nor were we expecting to rely on SIP integrations, but doesn't hurt to add the interface support

[c4-sponsor]

0xfoobar (sponsor) acknowledged

[GalloDaSballo]

Downgrading to QA as best practice, that won't cause a loss of funds

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[DadeKuma]

@GalloDaSballo C4 guidelines don't require a direct loss of funds for a valid medium:

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.

https://docs.code4rena.com/awarding/judging-criteria/severity-categorization#estimating-risk

This issue can definitely impact the protocol, as it can cause interoperability issues between contracts if they rely on SIP integrations.

It behaves very similarly to EIP-165 integrations, as consumers of this contract would check for the following:

The implementing contract MUST adhere to EIP-165 and provide a supportsInterface function that returns true for the interface of getSeaportMetadata(): 0x2e778efc. The contract MUST also return true for supportsInterface of the Seaport interface being implemented.

Implementing contracts MUST return a single schema ID for each additional SIP they support; they MUST NOT return any duplicate schema IDs and SHOULD return schema IDs in order from lowest to highest.

https://github.com/ProjectOpenSea/SIPs/blob/a368dc84061ea9922e4276569b97b03949e46dbd/SIPS/sip-5.md#specification

Failure to do so means that Delegate won't be supported by contracts that expect SIP compliance.

Moreover, I've asked the Seaport devs and this is definitely an issue that can negatively impact the protocol as the contract would also not be indexed on Seaport:

DadeKuma: Hey 0age! I'm working on a ContractOffererInterface implementation contract on Seaport, and I was skimming through the SIP documentation.

I was wondering if implementing SIP-5 (metadata standard) is required to avoid any issues.

This is the SIP I'm talking about: https://github.com/ProjectOpenSea/SIPs/blob/a368dc84061ea9922e4276569b97b03949e46dbd/SIPS/sip-5.md, which is currently a draft.

Do you think it's necessary to implement it right now, or is it better to avoid it, since it's a draft?

0age: Definitely want to implement it if you can so your contract can be indexed!

[0xfoobar]

We are not relying on any SIP interoperability beyond the onchain function calls, because there are no contracts implementing SIP. Does not have any impact on the protocol functioning or value leakage, this is clearly QA.

[DadeKuma]

I respectfully disagree with the sponsor, for the following reasons:

Similar to #260, this is not an issue regarding the interoperability between Delegate contracts, but it addresses the interoperability between Delegate and external contracts. The protocol will work fine in isolation.

I won't dispute further, and leave this as the judge's decision.

[GalloDaSballo]

Multiple points of contention: 1) No funds are at stake 2) No functionality is denied (See POC wrt to integrating with Seaport, no reverts due to this) 3) The SIP is a WIP as stated in the doc

The fun part The contract returns no Schema Which means it doesn't support any feature from Seaport