reNFT - SpicyMeatball's results

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L394-L400 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L406

Vulnerability details

Impact

Signer.sol is inherited by the Create and Stop policies where it's functions are used for the verification of the rental data. Unfortunately some of the type structs are encoded with violation of the EIP712 standards, this will result in in improper payload verification.

Proof of Concept

https://eips.ethereum.org/EIPS/eip-712#definition-of-encodetype According to EIP712 standard

If the struct type references other struct types (and these in turn reference even more struct types), then the set of referenced struct types is collected, sorted by name and appended to the encoding. An example encoding is Transaction(Person from,Person to,Asset tx)Asset(address token,uint256 amount)Person(address wallet,string name).

orderMetadataTypeHash is constructed with violation of this rule https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L406

bytes memory orderMetadataTypeString = abi.encodePacked(
    "OrderMetadata(uint8 orderType,uint256 rentDuration,Hook[] hooks,bytes emittedExtraData)"
);
orderMetadataTypeHash = keccak256(orderMetadataTypeString);

as we can see that the Hook struct was not included in the hash

rentPayloadTypeHash includes referenced structs orderMetadata and orderFulfillment in the hash, however they were not sorted by name https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L394-L400

            bytes memory rentPayloadTypeString = abi.encodePacked(
                "RentPayload(OrderFulfillment fulfillment,OrderMetadata metadata,uint256 expiration,address intendedFulfiller)"
            );

            // Derive RentPayload type hash via combination of relevant type strings.
            rentPayloadTypeHash = keccak256(
                abi.encodePacked(
                    rentPayloadTypeString,
                    orderMetadataTypeString,
                    orderFulfillmentTypeString
                )
            );

Tools Used

Manual review

Recommended Mitigation Steps

Encode rentPayloadTypeHash and orderMetadataTypeHash with respect to EIP712 rules

            // Derive RentPayload type hash via combination of relevant type strings.
            rentPayloadTypeHash = keccak256(
                abi.encodePacked(
                    rentPayloadTypeString,
                    orderFulfillmentTypeString,
                    orderMetadataTypeString
                )
            );

            // Derive the OrderMetadata type hash using the corresponding type string.
            orderMetadataTypeHash = keccak256(abi.encode(orderMetadataTypeString, hookTypeString));
        }

Assessed type

en/de-code