Renters can extend rental duration and hold lender's tokens hostage if ERC-777 is used as payment in a `PAY` order

[c4-bot-5]

Lines of code

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

Vulnerability details

Impact

The PaymentEscrow contract serves as an escrow for rental payments while rentals are active and handles their transfer when they are stopped. PAY orders can be stopped by the lender at any time, in which case the payment is split pro rata among the lender and the renter, and by anybody once the rental has expired, in which case the payment goes in full to the renter.

However, if an ERC-777 token is used as payment in a PAY order and the renter is a contract address, the renter can extend the duration of the rental until they receive the rental in full by reverting on the tokensReceived() hook until the full duration is over. This will cause the call to transfer() in the _safeTransfer() function to revert and prevent the lender from stopping the rental: https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/modules/PaymentEscrow.sol#L100-L119

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)
    );
    /*...*/
    if (!success || (data.length != 0 && !abi.decode(data, (bool)))) {
        revert Errors.PaymentEscrowModule_PaymentTransferFailed(token, to, value);
    }
}

The renter can be a contract as, per the sponsor:

Since the renter is the one doing the fulfilling and calling seaport, and the renter will be the offerer of the PAYEE order, it means that the renter does not have to sign this order. it can just be passed in.

And anyway:

Seaport supports signing orders with smart contracts via eip-1271

Furthermore, the renter could potentially extort the lender for larger amounts, as they can hold the NFTs hostage for as long as they want at no cost.

Proof of Concept

1. Alice (the lender) creates a PAY order with an ERC-777 token as payment.
2. Bob (the renter), a contract address, fulfills the order.
3. Bob reverts on the tokensReceived() hook, extending the duration of the rental.
4. Alice is unable to stop the rental until the full duration is over.
5. Bob can hold the NFTs hostage, potentially extorting Alice for larger amounts.

Tools Used

Manual review

Recommended Mitigation Steps

One possible mitigation would be to implement a mechanism to allow the renter to recuperate their items once a PAY rental has ended even if the transfer to the renter fails. Alternatively, an allowance could be set and execution resumed if a transfer fails, so that the recipient can pull the transfer themselves.

Assessed type

ERC20

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[c4-judge]

0xean marked the issue as duplicate of #65

[c4-judge]

0xean marked the issue as satisfactory

[c4-judge]

0xean changed the severity to 3 (High Risk)

[c4-judge]

0xean changed the severity to 2 (Med Risk)