reNFT - Audinarey's results

Lines of code

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

Vulnerability details

IMPACT

Protocol is not EIP712 compliant, which will result in issues with integrators. (See Coded POC for proof)

VULNERABILITY DETAILS

When implementing EIP712, among other things, for data structures that will be a part of signing message, a typehash must be defined. The structure typehash https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct is defined as

typeHash = keccak256(encodeType(typeOf(s)))

Where encodeType https://eips.ethereum.org/EIPS/eip-712#definition-of-encodetype is defined as name ‖ "(" ‖ member₁ ‖ "," ‖ member₂ ‖ "," ‖ … ‖ memberₙ ")" where each member is written as type ‖ " " ‖ name

---

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.

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)

The protocol uses several structs like

struct RentPayload {
    OrderFulfillment fulfillment; 
    OrderMetadata metadata; 
    uint256 expiration;
    address intendedFulfiller;
}

and

struct RentalOrder {
    bytes32 seaportOrderHash;
    Item[] items;
    Hook[] hooks;
    OrderType orderType;
    address lender;
    address renter;
    address rentalWallet;
    uint256 startTimestamp;
    uint256 endTimestamp;
}

in the protocol, the precalculated typehash for each of the structure are of different structures and evaluated in the Signer::_deriveRentalTypehashes()function

• for rentalOrderTypeHash,

// Construct the Item type string.
        bytes memory itemTypeString = abi.encodePacked(
            "Item(uint8 itemType,uint8 settleTo,address token,uint256 amount,uint256 identifier)"
        );

        // Construct the Hook type string.
        bytes memory hookTypeString = abi.encodePacked(
            "Hook(address target,uint256 itemIndex,bytes extraData)"
        );

        bytes memory rentalOrderTypeString = abi.encodePacked(
            "RentalOrder(bytes32 seaportOrderHash,Item[] items,Hook[] hooks,uint8 orderType,address lender,address renter,address rentalWallet,uint256 startTimestamp,uint256 endTimestamp)"
        );

        // Derive the Item type hash using the corresponding type string.
        itemTypeHash = keccak256(itemTypeString);

        // Derive the Hook type hash using the corresponding type string.
        hookTypeHash = keccak256(hookTypeString);

        // Derive the RentalOrder type hash using the corresponding type string.
        rentalOrderTypeHash = keccak256(
            abi.encode(rentalOrderTypeString, hookTypeString, itemTypeString)
        );

• for rentPayloadTypeHash

{
            // Construct the OrderFulfillment type string.
            bytes memory orderFulfillmentTypeString = abi.encodePacked(
                "OrderFulfillment(address recipient)"
            );

            bytes memory orderMetadataTypeString = abi.encodePacked(
                // @audit misses appernding the Hook
                "OrderMetadata(uint8 orderType,uint256 rentDuration,Hook[] hooks,bytes emittedExtraData)"
            );

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

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

            // Derive the OrderFulfillment type hash using the corresponding type string.
            orderFulfillmentTypeHash = keccak256(orderFulfillmentTypeString);

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

The type hashes shown aboveare used in the reNFT protocol and are not properly evaluated as shown above.

For instance, the orderMetadataTypeString does not append the Hook struct in line with the EIP712 standard

struct Hook {
address target,
uint256 itemIndex,
bytes extraData
}

CODED POC

The test below can be verified using forge test --mt test_hashes -vvv

• create a file named SignerHelper.sol in the @src/packages/SignerHelper.sol and copy the code below this is used to get the output of the _deriveRentalTypehashes() function

// SPDX-License-Identifier: BUSL-1.1
pragma solidity ^0.8.20;

import {Signer} from "@src/packages/Signer.sol";

contract SignerHelper is Signer {

    function deriveRentalTypehashes() 
    public 
    pure
    returns (
            bytes32 itemTypeHash,
            bytes32 hookTypeHash,
            bytes32 rentalOrderTypeHash,
            bytes32 orderFulfillmentTypeHash,
            bytes32 orderMetadataTypeHash,
            bytes32 rentPayloadTypeHash
    ) {
            return _deriveRentalTypehashes();
    }
}

• Create a folder in the @test/ folder and name it packages
• create a file in the @test/packages/Signer.t.sol and name it Signer.t.sol and copy the code below into the Signer.sol file

// SPDX-License-Identifier: BUSL-1.1
pragma solidity ^0.8.20;
import {Test} from "forge-std/Test.sol";
import {SignerHelper} from "@src/packages/SignerHelper.sol";

contract SignerTest is Test {
    SignerHelper public signer;

    function setUp() public {
        signer =  new SignerHelper();
    }

    // Construct the hash in line with EIP712 standard
    function rental_Order_And_PayLoad_Typehash() public pure returns(bytes32 rentalTypeHash, bytes32 payloadTypeHash) {
        bytes memory rentalOrderTypeString = abi.encodePacked(
            "RentalOrder(bytes32 seaportOrderHash,Item[] items,Hook[] hooks,uint8 orderType,address lender,address renter,address rentalWallet,uint256 startTimestamp,uint256 endTimestamp)Hook(address target,uint256 itemIndex,bytes extraData)Item(uint8 itemType,uint8 settleTo,address token,uint256 amount,uint256 identifier)"
        );

        rentalTypeHash = keccak256(rentalOrderTypeString);

        bytes memory rentalPayloadTypeString = abi.encodePacked(
            "RentPayload(OrderFulfillment fulfillment,OrderMetadata metadata,uint256 expiration,address intendedFulfiller)OrderFulfillment(address recipient)OrderMetadata(uint8 orderType,uint256 rentDuration,Hook[] hooks,bytes emittedExtraData)Hook(address target,uint256 itemIndex,bytes extraData)"
        );

        payloadTypeHash = keccak256(rentalPayloadTypeString);
    }

    function test_hashes() public {
        // get the protocol's rentalOrderTypeHash
        (,, bytes32 rentalOrderTypeHash,,, bytes32 rentPayloadTypeHash) = signer.deriveRentalTypehashes();

        // get the typehashes according to EIP712 standard
        (bytes32 rentalTypeHash, bytes32 payloadTypeHash) = rental_Order_And_PayLoad_Typehash();

        // check if they are equal
        bool isHashEqual = rentalOrderTypeHash == rentalTypeHash;
        bool isRentalPayloadTypeHashEqual = rentPayloadTypeHash == payloadTypeHash;

        // confirm they are not equal
        assertEq(isHashEqual, false);
        assertEq(isRentalPayloadTypeHashEqual, false);

        emit log_named_bytes32("reNFT rentalOrderTypeHash", rentalOrderTypeHash);
        emit log_named_bytes32("EIP712 rentalOrderTypeHash", rentalTypeHash);
        emit log_named_bytes32("reNFT rentPayloadTypeHash", rentPayloadTypeHash);
        emit log_named_bytes32("EIP712 rentPayloadTypeHash", payloadTypeHash);

    }

    
}

TOOLS USED

Foundry

RECOMMENDATIONS

Modify the structure typehash (and tests) to point to the correct structures in line with EIP712 standard

Assessed type

Other