Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QA Report #118

Open
code423n4 opened this issue Apr 7, 2022 · 0 comments
Open

QA Report #118

code423n4 opened this issue Apr 7, 2022 · 0 comments
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

1. Low - No automation of seizeCollateral enables frontrunning avoidance

Impact

The seizeCollateral function allows a lender to receive the loan collateral if a borrowed does not repay before the end of the loan duration. But this function is not automated by a keeper network, so the lender must manually call this function. There will be instances where the lender does not call seizeCollateral until hours or days after the loan period has ended. In this event, the lender will call the seizeCollateral function and expect to receive the collateral. But the borrower can frontrun this with a repayAndCloseLoan call, paying back the loan before the collateral can be seized.

The seizeCollateral function can be called by a lender after the loan duration has passed. If this happens, the lender will expect to receive the collateral. If this call is frontrun, the lender will lose out on receiving the collateral and instead the borrower gets a free loan extension with the originally agreed upon interest rate. This is most problematic for short-term loans of a few days, because a loan extension of 1-2 days, perhaps during a weekend when there is no one available to execute the transaction on an EOA, would be a substantial duration extension to the loan. This could disincentivize lenders from lending in the future, especially for short term loans.

Proof of Concept

The seizeCollateral call can be called by the lender when the loan duration has ended
https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L253

But if the loan is repayed and closed earlier in the same block as the seizeCollateral call, the loan.closed value will be set to true
https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L240

The lender will only receive interest for the period of the loan PLUS the period after loan expiration, while the borrower gets a "free" loan extension. Because there is no keeper system in place, the lender must manually trigger this call when they noticed the loan has not been repayed.

Example scenario #2:

  1. Bob takes out a 3 day loan from Alex with a 1% interest rate
  2. Bob doesn't pay back the loan on time but has friends deep in the mempool
  3. Alex calls seizeCollateral on day 5, 2 days after loan expiry. He expects to receive the collateral as compensation for not receiving the loan repayment
  4. Bob's mempool friends help him frontrun the seizeCollateral call with his own repayAndCloseLoan call
  5. The seizeCollateral call from Alex reverts, but Alex receives loan repayment. Sadly, the loan repayment used the original interest rate terms for the loan, so Bob got a free loan extension. If the market interest rates have risen significantly since day 3, Alex lost value.

Tools Used

Manual analysis

Recommended Mitigation Steps

Consider integrating with the gelato network for automated keepers: https://www.gelato.network/

Another option is to create a function that seizes collateral for overdue loans and sends the collateral to the lender. This function should have the onlyOwner modifier and it will allow the Backed Protocol team to prevent loans from sitting expired for extended periods with the potential of free loan extensions for the borrowers.

@code423n4 code423n4 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Apr 7, 2022
code423n4 added a commit that referenced this issue Apr 7, 2022
@wilsoncusack wilsoncusack added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

2 participants