Submitted by stackachu, also found by albertwh1te, oakcobalt, EV_om, sin1st3r__, 0xc695, serial-coder, hash, rbserver, DeFiHackLabs, krikolkk, 0xabhay, evmboi32, hals, peanuts, Krace, lanrebayode77, KupiaSec, HSP, said, ladboy233, J4X, ZanyBonzy, 0xHelium, peter, 0xpiken, Qkite, marqymarq10, cccz, and holydevoti0n
When a rental is stopped, Stop.stopRent() transfers the rented NFT back from the renters Safe to the lenders wallet and transfers the ERC20 payments from the payment escrow contract to the respective recipients (depending on the type of rental, those can be the renter, the lender, or both).
To transfer the ERC20 payments, PaymentEscrow.settlePayment() is called.
PaymentEscrow.settlePayment() will use _safeTransfer() (via _settlePayment() and _settlePaymentProRata() or _settlePaymentInFull()) to transfer the ERC20 payments to the recipients:
If either the payment recipient or the payment escrow contract are blocklisted in the payment ERC20, the transfer will fail and _safeTransfer() will revert. In this case the rental is not stopped, the rented NFT will still be in the renters Safe, and the payment will still be in the payment escrow contract.
Blocklisting is implemented by several stablecoins issued by centralized entities (e.g. USDC and USDT) to be able to comply with regulatory requirements (freeze funds that are connected to illegal activities).
There are multiple scenarios that can have impact here:
A. The renter of a PAY order rental is blocklisted: Even if the renter is already blocklisted before making the rental, the rental can still start, but ending the rental will not be possible, so the lender loses the rented NFT (it will be stuck in the Safe) and at least temporarily loses access to the payment (it will be stuck in the payment escrow, but a protocol admin could recover it).
B. The lender of a BASE order rental is blocklisted: Ending the rental will not be possible, so the lender loses the rented NFT and the payment. However, it is unlikely that the lender has been blocklisted arbitrarily during the rental or wasnt aware of the blocklisting before the rental, so this scenario seems unlikely.
C. The payment escrow contract becomes blocklisted during the rental (if it were blocklisted before the rental, the rental couldnt start): In this case the lender loses the rented NFT and the payment is lost. However, it seems unlikely that the payment escrow contract becomes blocklisted.
Out of the scenarios listed above, scenario A has the highest impact. Anyone who is blocklisted by a ERC20 contract can grief any lender of a PAY order lending offer that offers this ERC20 as payment of their rental NFT. The attacker only has to pay the gas fee to start the rental to carry out this attack.
Scenarios B and C are less severe (due to low likelihood) but still relevant, as the blocklisting carried out in the external payment ERC20 contract causes the loss of the rental NFT.
Add blocklisting to the MockERC20 contract:
Add the following import to StopRent.t.sol:
Add the following test to StopRent.t.sol:
Now the PoC can be run with:
I see two ways to mitigate this:
Alec1017 (reNFT) confirmed
0xean (Judge) decreased severity to Medium
