Malicious users can frontrun borrowers trying to repay loans, causing DoS and possibly a loan default

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-backed/blob/e8015d7c4b295af131f017e646ba1b99c8f608f0/contracts/NFTLoanFacilitator.sol#L129-L226 https://github.com/code-423n4/2022-04-backed/blob/e8015d7c4b295af131f017e646ba1b99c8f608f0/contracts/NFTLoanFacilitator.sol#L230-L250

Vulnerability details

Impact

Attackers can listen for a borrower to call repayAndCloseLoan on a specific loanId, and frontrun their transaction with a call to lend, creating a new loan with an increased amount, causing the borrower's transaction to fail due to the new loanAmount being greater than initially anticipated.

This can cause many issues: a borrower may repeatedly find that they cannot repay and close their loan, and while the duration of the loan is extended on takeover, the borrower must keep retrying and/or try again later if they do not want to default on the loan.

If the underlying collateral is valuable enough (either originally in value, or it appreciated), an attacker would be motivated to continuously prevent the loan from being closed in an attempt to seize the collateral.

If the duration of the loan, minDurationSeconds, is low enough, the attacker can simply takeover the loan with a 10% more amount, and hope that the borrower will fail to repay and close the new loan that now has a new deadline so that they can call seizeCollateral.

A dedicated attacker can continue to deny the borrower from repaying their loan indefinitely. If the borrower is a regular user, they likely will not be able to keep up and end up defaulting.

Proof of Concept

A simple test suite can be written where the borrower calls repayAndCloseLoan on their outstanding loan, an attacker can see that txn in the mempool and submit a higher-priority one calling lend with 10% more amount, causing the call to repayAndCloseLoan to revert due to insufficient balance. If the borrower does not repay the loan before the new deadline, the attacker can call seizeCollateral and receive the collateralized NFT.

Tools Used

Manual code review, vscode

Recommended Mitigation Steps

Front-running mitigation methods such as decay periods (durations after which certain txns are not allowed / conditions are changed) could be helpful.

[wilsoncusack]

Don't think we plan to fix this, part of how the protocol works

[gzeoneth]

Valid POC with fairy high probability of user losing collateral in the low duration scenario.

[wilsoncusack]

Valid POC with fairy high probability of user losing collateral in the low duration scenario.

The loan can be repaid early, so the issue is to do with the 1% origination fee. Otherwise the improvement tx goes through and the borrower repays as normal, anyway. Only an issue when loan amount is increased. When they are trying to repay, attacker loans more. Borrower gets amount sub origination fee but owes total amount. Can be significant at large values.

IMO this is then the same as the issues for the loan amount being unbounded. There is nothing unique about the last minute nature of this attack? If someone wanted to prevent it via a large loan amount they could do so at any time? In general I doubt lenders will often being willing to risk the amounts required to make the origination fee difference so significant the borrower can't repay.

[gzeoneth]

Valid POC with fairy high probability of user losing collateral in the low duration scenario.

The loan can be repaid early, so the issue is to do with the 1% origination fee. Otherwise the improvement tx goes through and the borrower repays as normal, anyway. Only an issue when loan amount is increased. When they are trying to repay, attacker loans more. Borrower gets amount sub origination fee but owes total amount. Can be significant at large values.

IMO this is then the same as the issues for the loan amount being unbounded. There is nothing unique about the last minute nature of this attack? If someone wanted to prevent it via a large loan amount they could do so at any time? In general I doubt lenders will often being willing to risk the amounts required to make the origination fee difference so significant the borrower can't repay.

The last minute variant significantly reduce the attacker's cost of attack. For example if a borrower repay 5 minutes before the deadline and can sign a tx every 1 minute, the attacker only need to risk (1+10%)^5-1 = 61% of the loan amount for the attack. Given the typical collateral ratio of NFT loan the risk is minimal. This issue is un-related to the loan amount bound because it is still possible to happen if it is bounded. I suggest preventing any buyout attempt X minutes before deadline.

[wilsoncusack]

Every buyout restarts the loan duration? And buyouts do not prevent repayment? If a buyout without loan increases and a repayment are in the same block the repayment is unaffected: the amount required to repay is the exact same.

Am I missing something @gzeoneth?

[gzeoneth]

Every buyout restarts the loan duration? And buyouts do not prevent repayment? If a buyout without loan increases and a repayment are in the same block the repayment is unaffected: the amount required to repay is the exact same.

Am I missing something @gzeoneth?

I am referring to a buyout that increase loan amount which will not restart the loan duration. When the loan amount increased, repayment can be affected. I believe having the loan amount unbounded is insufficient to mitigate the issue, it would need to be fixed. As I mentioned, the attacker only need 61% buffer to launch an attack if the borrower repay in the last 5 minutes.

[gzeoneth]

I made a mistake during the initial triage which sponsor clarified loan duration will always increase upon buyout because lastAccumulatedTimestamp will reset in L192. Changing this to duplicate of #24.