reNFT - piyushshukla's results

Lines of code

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

Vulnerability details

Impact

An attacker can repeatedly call stopRent, initiating fund transfers before the state is updated. This allows the attacker to exploit reentrancy, potentially stealing more funds than the intended rental amounts.

Proof of Concept

In the stopRent function of Stop.sol contract. If a lender repeatedly calls stopRent, it triggers the _reclaimRentedItems function. The vulnerability arises because the STORE.removeRentals function, responsible for updating the state, is called after transferring funds in the _reclaimRentedItems function. This creates a window of opportunity for an attacker to exploit reentrancy and steal more funds than intended.

1. Lender calls stopRent multiple times, invoking _reclaimRentedItems with each call. 2 _reclaimRentedItems transfers funds back to the lender. 3 .However, the STORE.removeRentals function, which updates the state, is called after the fund transfer

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

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

Tools Used

manual

Recommended Mitigation Steps

Implement a reentrancy guard at the beginning of the _reclaimRentedItems function to prevent multiple calls before the state changes are finalized.

Assessed type

Reentrancy