reNFT - ladboy233's results

Lines of code

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

Vulnerability details

H-stop-rent-does-not-follow-check-effect-pattern-and-subject-to-reentrancy-renter-erc777-token-reentrancy

Proof of concept

Line of code

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

first we call

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

then we remove rental

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

In this report, we only focus on ERC777 token-reentrancy. Reentrant call

This is a list of ERC777 token

This is a list of ERC777 token that has sufficient liquidity.

Impact

Line of code

 	ESCRW.settlePayment(order);

This function will eventually call Line of code

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

        _safeTransfer(token, lender, lenderAmount);

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

the logic involves transferring erc777 token to lender or renter can implement token hook to start reentrancy

because when the order type is PAYEE, both lender and renter will receive the ERC777 token if the lender ends the rental early

when the order type is BASIC, payment goes to renter, so renter can receive the ERC777 token to start reentrancy.

Please refer

function tokensReceived(
        address payable operator,
        address from,
        address to,
        uint256 amount,
        bytes calldata data,
        bytes calldata operatorData
    ) external {

Hacker will reenter the stopRental function to remove the erc777 token multiple times from payment escrow when there are multiple rentals

Recommendation

Move STORE.removeRentals

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

Up before this line of code

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

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

H-stop-rent-does-not-follow-check-effect-pattern-and-subject-to-reentrancy-nft-lender-reentrancy

Proof of concept

stopRent code is here

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

First we call

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

then we remove rental

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

Impact

in this report, we only focus on nft-lender-reentrancy

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

This function will eventually call Line of Code

function _transferERC721(Item memory item, address recipient) private {
        IERC721(item.token).safeTransferFrom(address(this), recipient, item.identifier);
    }

    /**
     * @dev Helper function to transfer an ERC1155 token.
     *
     * @param item      Item which will be transferred.
     * @param recipient Address which will receive the token.
     */
    function _transferERC1155(Item memory item, address recipient) private {
        IERC1155(item.token).safeTransferFrom(
            address(this),
            recipient,
            item.identifier,
            item.amount,
            ""
        );
    }

this would trigger the fallback onERC1155Received

if (to.code.length > 0) {
            try IERC1155Receiver(to).onERC1155Received(operator, from, id, value, data) returns (bytes4 response) {
                if (response != IERC1155Receiver.onERC1155Received.selector) {
                    // Tokens rejected
                    revert ERC1155InvalidReceiver(to);
                }

and the recipient and re-enter the stop rental to withdraw nft and rental payment multiple times

this would be a practical issue when there are multiple rental is in place

consider the case

1. rental 1 is ongoing, the renter rent 1 piece of SAND LAND ERC1155 NFT, with 1000 USDC token in payment escrow
2. rental 2 is onging, the renter rent 3 piece of SAND LAND ERC1155 NFT, with 5000 USDC token in payment escrow
3. the 4 LAND NFT belongs to the same owner, the owner wants to terminate the rental term, and exploit reentry

this is the function call regular flow

2. check if the rental 1 term is active, check pass
3. compute how much rental refund goes to the renter and compute how much rental premium the lender can collect
4. transfer 800 USDC back to renter, transfer 200 USDC back to lender and the 1 SAND LAND NFT to lender
5. clear the rental state and delete the rental

the function call flow for re-entrancy

3. check if the rental 1 term is active, check pass
4. compute how much rental refund goes to the renter and compute how much rental premium the lender can collect
5. transfer 800 USDC back to renter, transfer 200 USDC back to lender and the 1 SAND LAND NFT to lender
6. when transfer 1 SAND LAND NFT to lender, onERC1155Received is triggered, we re-enter the function stop rental
7. check if the rental 1 term is active, check pass
8. compute how much rental refund goes to the renter and compute how much rental premium the lender can collect
9. transfer another 800 USDC back to renter, transfer 200 USDC back to lender and the 1 SAND LAND NFT to lender....

until all nft are transferred out, the transaction ends

renter may expects to use the 3 SAND NFT during rental term 2 but in this case, the lender malicious terminate term 1 while remove the nft (get back the nft that belongs to rental term 2)

Recommendation

follow check effect interaction and clear the rental storage first before sending nft out

Assessed type

Reentrancy

STORE.removeRentals(
    _deriveRentalOrderHash(order),
    _convertToStatic(rentalAssetUpdates)
);

Lines of code

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

Vulnerability details

M-User-can-still-execute-transaction-via-removed-module

Proof of concept

The Gnosis Safe wallet allows users to enable or disable modules. Line of code

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

However, users can only enable or disable modules that are whitelisted.

The implicit expectation is that users should execute transactions from a module if it is active and has been whitelisted by the admin.

And users should not execute transactions from a module if it is no longer whitelisted by the admin.

Impact

Even if the admin removes a module (extension),

users may choose not to disable the extension and can still execute transactions using a module that is not whitelisted.

there is lack of validation for calldata and calling method when the method execTransactionFromModule is called,

in case when a not whitelisted module get compromised, the hacker can transfer the nft or token out from the renter's gnosis safe wallet by calling execTransactionFromModule directly and bypass all check from the guard

Line of code

/**
     * @notice Execute `operation` (0: Call, 1: DelegateCall) to `to` with `value`
     *         (Native Token).
     *
     * @dev Function is virtual to allow overriding for L2 singleton to emit an event for
     *      indexing.
     *
     * @param to        Destination address of module transaction.
     * @param value     Ether value of module transaction.
     * @param data      Data payload of module transaction.
     * @param operation Operation type of module transaction.
     *
     * @return success  Boolean flag indicating if the call succeeded.
     */
    function execTransactionFromModule(
        address to,
        uint256 value,
        bytes calldata data,
        Enum.Operation operation
    ) external returns (bool success);

there is lack of check to validate if the function call triggered is execTransactionFromModule in the guard contract

Recommendation

add such check to make sure if the module is not whitelisted, execTransactionFromModule no longer works as well

Assessed type

call/delegatecall

Lines of code

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

Vulnerability details

M-blocklisted-renter-can-prevent-lender-from-execute-pay-order-to-get-nft-back

Proof of concept

some tokens such as USDT and USDC have a contract level admin controlled address blocklist

In PAYEE order type

The lender has the option to terminate the rental early.

Part of the rental payment may be paid to the renter, depending on the elapsed rental time.

In BASIC order type

after rental expires, the nft should be transferred back to lender and the renter receive the payment

Impact

However, if the rental is blacklisted, this means the rental payment to the renter would be reverted. In such a case, the NFT owner (lender) loses the option to terminate the rental or lose his

Line of code

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

function settlePayment

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

when the payment for the order settle, we call function _settlePayment

if (orderType.isPayOrder() && !isRentalOver) {
                    // Interaction: a PAY order which hasnt ended yet. Payout is pro-rata.
                    _settlePaymentProRata(
                        item.token,
                        paymentAmount,
                        lender,
                        renter,
                        elapsedTime,
                        totalTime
                    );
                }
                // If its a PAY order and the rental is over, or, if its a BASE order.
                else if (
                    (orderType.isPayOrder() && isRentalOver) || orderType.isBaseOrder()
                ) {
                    // Interaction: a pay order or base order which has ended. Payout is in full.
                    _settlePaymentInFull(
                        item.token,
                        paymentAmount,
                        item.settleTo,
                        lender,
                        renter
                    );
                } else {
                    revert Errors.Shared_OrderTypeNotSupported(uint8(orderType));
                }

If a pay order which hasnt ended. It called _settlePaymentProRata

which then will call _safetransfer

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

If a pay order which has ended. It called function _settlePaymentInFull which then will call _safetransfer

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

Recommendation

Renter can claim the payment token themselves instead of smart contract transfering the payment out to avoid the case when the renter address is blocklisted

Assessed type

Token-Transfer

Renter can lock or steal lender's nft if the nft used is moonbird

at the time of writing the bug report

moonbird nft, as one of the blue chip nft in last crypto bull run

https://opensea.io/collection/proof-moonbirds

has 343385 ETH trading volume

if the nft during the rental is moonbird,

the rental can maliciously prevent the lender from getting his nft back

after the rental period, the expect call flow is transferring nft back

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

this would eventually calls

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

but if the nft is moonbird, during the rental period

the owner can call the function toggleNesting

https://etherscan.io/token/0x23581767a106ae21c074b2276d25e5c3e136a68b#code#F1#L319

and once toggleNesting is enabled, the nft transfer is disabled,

meaning once the owner turn on the nesting, the original lender cannot transfer nft out

https://etherscan.io/token/0x23581767a106ae21c074b2276d25e5c3e136a68b#code#F1#L272

    function _beforeTokenTransfers(
        address,
        address,
        uint256 startTokenId,
        uint256 quantity
    ) internal view override {
        uint256 tokenId = startTokenId;
        for (uint256 end = tokenId + quantity; tokenId < end; ++tokenId) {
            require(
                nestingStarted[tokenId] == 0 || nestingTransfer == 2,
                "Moonbirds: nesting"
            );
        }
    }

the function that can transfer nft out is

https://etherscan.io/token/0x23581767a106ae21c074b2276d25e5c3e136a68b#code#F1#L258

safeTransferWhileNesting

in fact, the renter can steal lender's nft and bypass the guard check

the renter can just batch transaction via safe walllet by calling

1. toggleNesting
2. safeTransferWhileNesting

Should name the function that withdraw the fee explicitly

if the admin wants to withdraw the fee, the admin has to call the function skim

    /**
     * @notice Skims all protocol fees from the escrow module to the target address.
     *
     * @param token Token address which denominates the fee.
     * @param to    Destination address to send the tokens.
     */
    function skim(address token, address to) external onlyRole("ADMIN_ADMIN") {
        ESCRW.skim(token, to);
    }

it is recommend rename the function to a more explicit name such as "withdrawFee" instead of name the function is "skim"

Renter can claim nft airdrop onbehalf of original lender

when the order is fulfilled, the payment goes to payment escrow while the nft goes to the safe wallet

but during the rental term

if there are airdrop based on nft, the airdrop goes to the safe wallet

the owner of the safe wallet, which is renter, can claim the airdrop before lender reclaim the nft ownership.

if the recipient is not capable of receiving nft, both nft are locked

when the rental term ends,

the function call that transfer the nft back to the original lender is

does not support ETH native asset as payment

the protocol support ERC721 token, ERC1155 token, and ERC20 in offer and consideration

but in seaport protocol

if the asset field is address(0), it indicate the order can be settled with native ETH token,

it is recommend that the renft protocol also support order settlement in native ETH token

Renter can still transfer out if the nft use ERC1155A stand

once the nft is in the safe wallet, no one should transfert the nft out

but if the nft inherit and use ERC1155A standard

the owner can craft payload to call

https://github.com/superform-xyz/ERC1155A/blob/f46fa542026b860717d48bd1c09acbb8b68a0b57/src/interfaces/IERC1155A.sol#L96

    /// @notice Public function for setting single id approval
    /// @dev Notice `owner` param, it will always be msg.sender, see _setApprovalForOne()
    /// @param spender address of the contract to approve
    /// @param id id of the ERC1155A to approve
    /// @param amount amount of the ERC1155A to approve
    function setApprovalForOne(address spender, uint256 id, uint256 amount) external;

    /// @notice Public function for setting multiple id approval
    /// @dev extension of sigle id approval
    /// @param spender address of the contract to approve
    /// @param ids ids of the ERC1155A to approve
    /// @param amounts amounts of the ERC1155A to approve
    function setApprovalForMany(address spender, uint256[] memory ids, uint256[] memory amounts) external;

    /// @notice Public function for increasing single id approval amount
    /// @dev Re-adapted from ERC20
    /// @param spender address of the contract to approve
    /// @param id id of the ERC1155A to approve
    /// @param addedValue amount of the allowance to increase by 
    function increaseAllowance(address spender, uint256 id, uint256 addedValue) external returns (bool);

to call setApprovalForOne, setApprovalForMany or increaseAllowance to approve external contract, then control external contract to transfer nft out

superform nft is one of the nft that use ERC1155A standard

loss of rebase token in payment escrow

if the token has rebase behavior such as stETH or USDY, and if there is positive rebase

the additional rebased token is locked in the payment escrow contract and lost

Low level function call does not check token existence when settle the payment

when the payment escrow settle token transfer, the function uses low level call

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

    function _safeTransfer(address token, address to, uint256 value) internal {
        // Call transfer() on the token.
        (bool success, bytes memory data) = token.call(
            abi.encodeWithSelector(IERC20.transfer.selector, to, value)
        );

        // Because both reverting and returning false are allowed by the ERC20 standard
        // to indicate a failed transfer, we must handle both cases.
        //
        // If success is false, the ERC20 contract reverted.
        //
        // If success is true, we must check if return data was provided. If no return
        // data is provided, then no revert occurred. But, if return data is provided,
        // then it must be decoded into a bool which will indicate the success of the
        // transfer.
        if (!success || (data.length != 0 && !abi.decode(data, (bool)))) {
            revert Errors.PaymentEscrowModule_PaymentTransferFailed(token, to, value);
        }
    }

but if the token is self-destructed and no longer exists, no token transfer is done but the payment is still settled

it is recommend to use openzeppelin safeTransfer, it has built-in contract existence check by default

No one can stop and settle the rental if the lender address does not implement onERC721Received or onERC1155Received

when the rental is settled, someone needs to call stop rental, this involves transferring nft back to the lender

    function _transferERC721(Item memory item, address recipient) private {
        IERC721(item.token).safeTransferFrom(address(this), recipient, item.identifier);
    }

    /**
     * @dev Helper function to transfer an ERC1155 token.
     *
     * @param item      Item which will be transferred.
     * @param recipient Address which will receive the token.
     */
    function _transferERC1155(Item memory item, address recipient) private {
        IERC1155(item.token).safeTransferFrom(
            address(this),
            recipient,
            item.identifier,
            item.amount,
            ""
        );
    }

the method used is safeTransferFrom

this requires a recipient implement onERC721Received or onERC1155Received method

so when the recipient (lender) does not implement this method, the rental failed to settle and nft is locked in the safe wallet and payment is locked in the payment escrow

reNFT Audit Details Analysis Report

Introduction

reNFT is a protocol that enables generalized collateral-free NFT rentals, utilizing Gnosis Safe and Seaport frameworks. This report analyzes the various aspects of the protocol, including its security features, module functions, policy mechanisms, and potential vulnerabilities based on the provided documentation.

The reNFT protocol presents an innovative approach to NFT rentals, utilizing existing frameworks like Gnosis Safe and Seaport. However, it faces challenges in ensuring the security of rental wallets and handling various token standards. The audit identifies key areas of concern and suggests potential improvements to enhance the protocol's robustness and reliability. As the protocol evolves, continuous monitoring and updating of security measures will be crucial in maintaining its integrity and user trust.

Key Components

1. Modules: These are internal-facing contracts that store shared state across the protocol. Key modules include Payment Escrow and Storage.

2. Policies: External-facing contracts that handle inbound calls and route updates to data models via Modules. Notable policies include Admin, Create, Factory, Guard, and Stop.

3. Packages: Helper contracts like Accumulator, Reclaimer, and Signer assist in specific tasks within the protocol.

4. General Contracts: Contracts like Create2 Deployer and Kernel manage a variety of functionalities, including registry and policy management.

Security Considerations

1. Manipulation via Hook Contracts: Hook contracts, serving as middleware, can potentially execute arbitrary logic, posing risks if malicious or faulty hooks are used.

2. Dishonest or Malicious ERC721/ERC1155 Implementations: The protocol can't prevent transfers via dishonest implementations that deviate from the standard ERC721/ERC1155 specifications.

3. Rebasing or Fee-On-Transfer ERC20 Implementations: The protocol doesn’t account for ERC20 tokens with fee-on-transfer or rebasing features, which can alter balances unexpectedly.

Protocol Mechanics

1. Rental Process: The protocol facilitates NFT rentals where assets are transferred to a Gnosis safe created for the renter, with restrictions on asset transfer to ensure security.

2. Payment Escrow: Manages rental payments, ensuring proper distribution post-rental period.

3. Admin Operations: Include fee, proxy, and whitelist management, critical for maintaining the protocol’s integrity.

4. Deployment and Registry Management: Ensured by Create2 Deployer and Kernel contracts, respectively.

Potential Attack Vectors

1. Rental Wallet Security: Ensuring rented assets can't be freely moved by the rental wallet is a primary concern.

2. Rental Creation and Stopping: Ensuring accurate logging and handling of rentals to prevent unauthorized asset movement or payment discrepancies.

Centralization Risk and System Risk

• Backend Server Owner's Signature: A notable centralization risk arises from the requirement for the backend server owner to sign the validateOrder callback. This design choice introduces a potential single point of failure and undermines the decentralized nature of the system.
• Dependency on Order Matching: The system's reliance on the availability of a counterparty to fulfill orders presents a significant risk. In cases where no counterparty is interested, NFT lenders are unable to generate yield, leading to potential inefficiencies in the marketplace.

Recommendations

Based on the analysis, the following high-level recommendations are made:

• Enhance Decentralization: Reconsider the design requiring the backend server owner’s signature to mitigate centralization risk.
• Improve Order Matching Mechanisms: Develop strategies to ensure efficient order matching and mitigate the risk of unfulfilled orders.
• Secure Airdrop Claiming Process: Implement measures to ensure that airdrops reach the intended recipient, the original lender.
• Expand Payment Options: Introduce native ETH token support to increase the protocol's adaptability and user convenience.
• Strengthen NFT Transfer Security: Address vulnerabilities specific to different NFT standards, such as ERC1155A, to prevent unauthorized transfers.
• Handle Rebase Tokens Appropriately: Modify the payment escrow mechanism to account for and manage rebase tokens effectively.

Enhanced Security Measures for Hook Contracts: Implementing robust validation and authorization mechanisms for hook contracts could mitigate potential exploits.

Handling Non-Standard ERC721/ERC1155 Tokens: Developing strategies to detect and manage non-standard implementations could enhance security.

Accommodating Fee-On-Transfer and Rebasing ERC20 Tokens: Adjusting the protocol to handle these types of ERC20 tokens can prevent balance discrepancies.

Conclusion

The codebase exhibits several areas of concern, primarily related to centralization risks, system inefficiencies, and specific technical loopholes in NFT handling. Addressing these issues is crucial for the system's security, efficiency, and overall trustworthiness in the market.

Time spent:

35 hours