This is a big issue as it would not let the lender get back the NFT.
Proof of Concept
When finishing a PAY order, an order for which the lender is paying the borrower. The stopRent(...) function is in charge of transferring the NFT to the lender and the paiement to the borrower.
Its logic is the folllowing:
Transfer the NFT back to the lender
Settle the payment to the borrower and lender
For some famous tokens like USDT or USDC, the borrower could be a blacklisted address. In the context of a PAY order the borrower is the one receiving the payment.
In order to settle the paiement at the end (or during) a PAY order, the protocol is using the safeTransfer(...) function of the ERC20 standard. This function will check that the payment did not fail and will revert if it did.
EVM blockchains being atomic, if the payment fails, the NFT transfer will also fail and the NFT will not be transferred back to the lender.
It would be stuck in the borrower Safe.
Tools Used
Manual review
Recommendation
use the normal transfer function of the ERC20 standard
Lines of code
https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Stop.sol#L265-L306
Vulnerability details
Impact
This is a big issue as it would not let the lender get back the NFT.
Proof of Concept
When finishing a
PAY
order, an order for which the lender is paying the borrower. ThestopRent(...)
function is in charge of transferring the NFT to the lender and the paiement to the borrower. Its logic is the folllowing:For some famous tokens like USDT or USDC, the borrower could be a blacklisted address. In the context of a
PAY
order the borrower is the one receiving the payment. In order to settle the paiement at the end (or during) a PAY order, the protocol is using the safeTransfer(...) function of the ERC20 standard. This function will check that the payment did not fail and will revert if it did. EVM blockchains being atomic, if the payment fails, the NFT transfer will also fail and the NFT will not be transferred back to the lender.It would be stuck in the borrower Safe.
Tools Used
Manual review
Recommendation
Assessed type
ERC20