Backed Protocol contest - 0xkatana's results

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.

1. Short require strings save gas

Impact

Strings in solidity are handled in 32 byte chunks. A require string longer than 32 bytes uses more gas. Shortening these strings will save gas.

Proof of Concept

Several cases of this gas optimization were found. These are a few examples, but more may exist

1. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L84
2. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L86
3. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L121
4. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L146-L148
5. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L171-L172
6. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L178
7. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L189
8. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L255
9. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L259

Tools Used

Manual analysis

Recommended Mitigation Steps

Shorten require strings

2. Use != 0 instead of > 0

Impact

Using > 0 uses slightly more gas than using != 0. Use != 0 when comparing uint variables to zero, which cannot hold values below zero

Proof of Concept

There were two instances of this issue https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L198 https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L321

Tools Used

grep

Recommended Mitigation Steps

Replace > 0 with != 0 to save gas

3. Add payable to functions that won't receive ETH

Impact

Identifying a function as payable saves gas. Functions that have the onlyOwner modifier cannot be called by normal users and will not mistakenly receive ETH. These functions can be payable to save gas.

Proof of Concept

There are many functions that have the onlyOwner modifier and can be payable setLendTicketContract https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L279 setBorrowTicketContract https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L289 withdrawOriginationFees https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L296 updateOriginationFeeRate https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L306 updateRequiredImprovementRate https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L320

Tools Used

Manual analysis

Recommended Mitigation Steps

Add payable to these functions for gas savings

4. Use Solidity errors instead of require

Impact

Solidity errors introduced in version 0.8.4 can save gas on revert conditions https://blog.soliditylang.org/2021/04/21/custom-errors/ https://twitter.com/PatrickAlphaC/status/1505197417884528640

Proof of Concept

Some error messages are used, but many require blocks still exist in the code. A few examples are

1. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L84
2. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L86
3. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L121
4. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L146-L148
5. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L171-L172
6. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L178
7. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L189
8. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L255
9. https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanFacilitator.sol#L259

Tools Used

Manual analysis

Recommended Mitigation Steps

Replace require blocks with new solidity errors described in https://blog.soliditylang.org/2021/04/21/custom-errors/

5. Public functions can be external

Impact

Declaring a function as external instead of public saves gas

Proof of Concept

One function can be declared external instead of public

• tokenURI function https://github.com/code-423n4/2022-04-backed/blob/main/contracts/NFTLoanTicket.sol#L38

Tools Used

Manual analysis

Recommended Mitigation Steps

Change function from public to external to save gas