c4-bot-5 commented 10 months ago

Lines of code

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

Vulnerability details

Impact

ERC20 blacklist user can fulfill PAY order and make ERC721/ERC1155 items locked in the Safe.

Proof of Concept

When a rent is stopped, Reclaimer#reclaimRentalOrder is delegate called from Safe to transfer ERC721/ERC1155 items to lender:

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

And PaymentEscrow#settlePayment is called to transfer ERC20 tokens to renter:

        // Send the payment.
        _safeTransfer(token, settleToAddress, amount);

Transaction will revert if transfer failed:

        if (!success || (data.length != 0 && !abi.decode(data, (bool)))) {
            revert Errors.PaymentEscrowModule_PaymentTransferFailed(token, to, value);
        }

Some tokens (e.g. USDC, USDT) have a contract level admin controlled address blocklist. If an address is blocked, then transfers to and from that address are forbidden.

If a ERC20 blacklisted user fulfilled a PAY order, then order cannot be stopped due to transferring ERC20 tokens to the renter will always revert, the lender's ERC720/ERC1155 items will be locked in the Safe.

Tools Used

Manual Review

Recommended Mitigation Steps

Do not directly transfer ERC20 tokens to renter, renter need to claim themselves.

Assessed type

DoS

c4-pre-sort commented 9 months ago

141345 marked the issue as duplicate of #64

c4-judge commented 9 months ago

0xean changed the severity to 2 (Med Risk)