reNFT - 0xPsuedoPandit's results

Lines of code

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

Vulnerability details

Impact

Signer::_deriveRentalTypehashes will obtain wrong protocol specific type hashes.

Proof of Concept

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 is defined as: typeHash = keccak256(encodeType(typeOf(s)))

Where encodeType is the type of a struct that is encoded as: name ‖ "(" ‖ member₁ ‖ "," ‖ member₂ ‖ "," ‖ … ‖ memberₙ ")"

And each member is written as: type ‖ " " ‖ name.

Out of many structs protocol uses the signature of two structs that is OrderMetadata and Item defined in RentalStructs::_deriveRentalTypehashes

struct OrderMetadata { // Type of order being created. OrderType orderType; // Duration of the rental in seconds. uint256 rentDuration; // Hooks that will act as middleware for the items in the order. Hook[] hooks; // Any extra data to be emitted upon order fulfillment. bytes emittedExtraData; }

struct Item { ItemType itemType; SettleTo settleTo; address token; uint256 amount; uint256 identifier; }

The problem is while calculating their hashes there are typos that will create different hashes. In both the structs, pointing out the exact elements of structs,ItemType,SettleTo and OrderType is stuct in itself, but their hashes were calculated as uint8 orderType ,uint8 itemType and unit8 settleTo. https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L351-L354 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L383-L386 which creates a mismatch between the intended and actual results. paste this test case in any unit test file.

function testValidateStructs() public { vm.expectRevert("Item hash does not match"); assertEq( keccak256( abi.encodePacked( "Item(uint8 itemType,uint8 settleTo,address token,uint256 amount,uint256 identifier)" ) ), keccak256( abi.encodePacked( "Item(ItemType itemType,SettleTo settleTo,address token,uint256 amount,uint256 identifier)" ) ) );

vm.expectRevert("OrderMetadata hash does not match");

assertEq(
    keccak256(
        abi.encodePacked(
            "OrderMetadata(uint8 orderType,uint256 rentDuration,Hook[] hooks,bytes emittedExtraData)"
        )
    ),
    keccak256(
        abi.encodePacked(
            "OrderMetadata(OrderType orderType,uint256 rentDuration,Hook[] hooks,bytes emittedExtraData)"
        )
    )
);

Tools Used

Manual Review

Recommended Mitigation Steps

Modify the structure typehash (and tests) to point to the correct structures.

Assessed type

en/de-code