reNFT - 0xabhay's results

Lines of code

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

Vulnerability details

Impact

The lack of a check on the size of the rentalAssets array in the Accumulator.sol::_insert function can lead to denial of service (DoS) attacks. An attacker could repeatedly call a function that uses _insert, causing the array to grow until the transaction consumes all available gas and fails. This could prevent legitimate users from successfully interacting with the contract, as their transactions may also run out of gas or become prohibitively expensive due to the increased computational overhead.

Proof of Concept

the Create.sol::validateOrder function is part of a contract that interacts with a rental system, possibly within a marketplace like Seaport. The function decodes a RentPayload from the extraData parameter, performs various checks, and if all checks pass, it calls Create.sol::_rentFromZone to handle the rental logic. The _rentFromZone function iterates over a list of items and, for each item that is a rental, calls the Accumulator.sol::_insert function to add a RentalAssetUpdate to the rentalAssetUpdates dynamic array.

The potential vulnerability lies in the _insert function, which does not check the size of the rentalAssetUpdates array, potentially leading to out-of-gas errors if the array grows too large

Setup: An attacker identifies a contract that uses the validateOrder function to process rental orders. This function, upon successful validation, calls _rentFromZone, which in turn calls _insert to add rental updates to an array.

Payload Crafting: The attacker crafts a valid RentPayload and corresponding signature that will pass all the checks within validateOrder. This includes ensuring the signature has not expired, the fulfiller is correct, and the signer has the appropriate role.

Repeated Execution: The attacker then repeatedly calls validateOrder with the crafted payload. Each call processes a rental order and adds a new entry to the rentalAssetUpdates array via the _insert function.

Unchecked Growth: Since _insert does not check the size of the rentalAssetUpdates array, the attacker can continue to add entries until the transaction consumes all available gas.

Resulting Denial of Service: Eventually, the gas cost for processing an additional entry exceeds the transaction's gas limit, causing it to fail. If the attacker can automate and sustain this process, it could lead to a denial of service, where legitimate users are unable to process their rental orders due to out-of-gas errors or prohibitively high gas costs.

Tools Used

Manual review

Recommended Mitigation Steps

Limiting the Size of the Array: Introduce a maximum size for the rentalAssets array to prevent it from growing indefinitely.

Requiring Fees for Orders: Charge a fee for processing orders to disincentivize spamming and reduce the risk of DoS attacks.

Gas Checks: Implement checks to ensure that the gas usage of the validateOrder and rentFromZone functions does not approach the block gas limit

Assessed type

DoS

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#L320-L330 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L271 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L201 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L100-L118

Vulnerability details

Impact

The inability to settle payments due to a Lender or Renter being blacklisted on the ERC20 contract. (e.g. USDC, USDT)

Proof of Concept

A lender who is blacklisted by the USDT contract creates a rental agreement where USDT is the consideration(Base Order). A renter fulfills the order, and the USDT is transferred to the PaymentEscrow.sol contract. Upon rental expiration, the PaymentEscrow attempts to transfer USDT to the lender. The transfer fails due to the blacklist restriction in the USDT contract, and the transaction reverts.

   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);
    }

The PaymentEscrow::_settlePaymentInFull function is intended to transfer the specified amount of the token (in this case, USDT) to either the lender or the renter, depending on the settleTo parameter. If the settleTo is SettleTo.LENDER, the payment will be sent to the lender's address.

However, if the lender's address is blacklisted by the USDT contract, the _safeTransfer call within _settlePaymentInFullwill fail. This is because the USDT contract will likely have a modifier or require statement that checks if the recipient is blacklisted and, if so, reverts the transaction.

When the transaction reverts, the state changes made by the transaction are rolled back, and the USDT tokens would not be transferred from the PaymentEscrow to the lender. This would leave the system in a state where the rental agreement's financial settlement cannot be completed as intended. Also, prevent the rental from stopping even if it has expired.

Note: This bug can repeat if the renter's address is blacklisted. Similarly, if it is a pay order and the renter is blacklisted, the _settlePaymentInFull function won't be able to transfer the token.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a verification step to check if the lender's or renter address is blacklisted before creating a rental Order.

Assessed type

Other