`OrderMetadata` encoding breaks EIP-712 compliance

[c4-bot-7]

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L231-L238

Vulnerability details

Impact

According to the EIP-712 specification, structs are encoded by taking the hash of the concatenation of their type hash and the encoding of each of their fields. However, the _deriveOrderMetadataHash() function in the Signer contract is incorrect in that it omits one of the struct's fields, emittedExtraData, when computing the hash of the OrderMetadata struct.

Proof of Concept

The OrderMetadata struct is defined as follows:

struct OrderMetadata {
    OrderType orderType;
    uint256 rentDuration;
    Hook[] hooks;
    bytes emittedExtraData;
}

But the _deriveOrderMetadataHash() returns: https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L232-L238

    keccak256(
        abi.encode(
            _ORDER_METADATA_TYPEHASH,
            metadata.rentDuration,
            keccak256(abi.encodePacked(hookHashes))
        )
    );

The emittedExtraData field is left out when computing the hash of the OrderMetadata. This breaks EIP-712 compliance, so can be seen as an instance of “function of the protocol or its availability could be impacted” and is hence classified as medium severity.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this issue, the _deriveOrderMetadataHash() function should be updated to include the emittedExtraData field when computing the hash of the OrderMetadata struct. This will ensure that the function is in compliance with the EIP-712 specification and that the hash accurately represents the OrderMetadata struct.

Assessed type

Other

[c4-pre-sort]

141345 marked the issue as duplicate of #239

[c4-judge]

0xean marked the issue as satisfactory