xiaoming90 - EIP 712 is not implemented for the Edition contract.

[sherlock-admin3]

xiaoming90

medium

EIP 712 is not implemented for the Edition contract.

Summary

EIP 712 is not implemented for the Edition contract.

Vulnerability Detail

The Contest's README stated that EIP 712 must be strictly implemented for the Edition contract. Thus, in the context of this audit contest, any non-compliance is considered a valid Medium issue. Note that the Contest's README supersedes Sherlock's judging rules per Sherlock's Hierarchy of truth.

Is the codebase expected to comply with any EIPs? Can there be/are there any deviations from the specification?

strict implementation of EIPs
1271 (Graph), 712 (Graph, Edition), 2981 (Edition), 1155 (Edition)

However, when reviewing the Edition contract, it was observed that EIP 712 is not implemented for the Edition contract. Thus, breaking the above requirement.

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L36

File: Edition.sol
34: /// @title Edition
35: /// @notice An ERC1155 contract representing a collection of related works. Each work is represented by a token ID.
36: contract Edition is IEdition, ERC1155, ERC2981, Initializable, OwnableRoles {
37:     using SafeTransferLib for address;
..SNIP..

Impact

EIP 712 is not implemented for the Edition contract.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/editions/Edition.sol#L36

Tool used

Manual Review

Recommendation

Consider implementing EIP 712 for the Edition contract.

[pqseags]

This seems valid based on the Readme

[Hash01011122]

Seems borderline low/medium

[0x73696d616f]

Escalate

Should be low as it does not have any impact. EIP712 issues are valid when it causes problems due to not being able to correctly sign signatures, but this is not the case here.

[sherlock-admin3]

Escalate

Should be low as it does not have any impact. EIP712 issues are valid when it causes problems due to not being able to correctly sign signatures, but this is not the case here.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[realfugazzi]

Agree with invalidation reason, looks like a feature request.

[cducrest]

EIP712 does not define an interface contract or anything in the like, it is about hashing and signing typed structured data. This issue does not indicate any non-compliance with EIP712. It seems to be pointed that the contract does not inherit from an IEIP712 interface or the like, which is not a problem considering EIP712.

See https://eips.ethereum.org/EIPS/eip-712.
See also Solady's implementation of EIP712 which defines only one public function eip712Domain() defined in EIP5267 and not EIP712: https://github.com/vectorized/solady/blob/main/src/utils/EIP712.sol
https://eips.ethereum.org/EIPS/eip-5267

[WangSecurity]

Agree with the escalation. Planning to accept it and invalidate the issue, unless someone shows and proves how it effects external integrations.

[Hash01011122]

Agree with the escalation this is indeed low

[Evert0x]

Result:
Invalid
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• 0x73696d616f: accepted