Beedle - Oracle free perpetual lending

Summary

The borrow function in Lender.sol can cause denial of service. Also Lender.repay() and Lender.giveLoan() and Lender.startAuction() and seizeLoan() and refinance() can cause DOS

Vulnerability Details

There is a comment above the borrow() function in Lender.sol :

"/// you are allowed to open many borrows at once"

The function takes an array of borrow struct as input. Now in theory this is a potential unbounded array passed in as an input but in reality this is not likely to happen as the user has to provide a collateral for every borrow , therefore large amounts of funds will be needed to make this array have many iterations. Additionally the check
if (debt < pool.minLoanSize) revert LoanTooSmall(); prevents this

However the code that gets executed on every iteration of the array is too large which only contributes to the possibility of exceeding the block's gas limit. Actually the whole Lender.borrow(Borrow[] calldata borrows) function logic is executed on every iteration of the loop.

The situation is the same in the following two functions - repay() and giveLoan().They as well take arrays as inputs (giveLoan() even takes two array inputs).And the whole logic of the function is executed on every iteration of the loop.

In startAuction() again an array of uint256 (loanIds) is passed as input and the function logic is executed on every iteration of the loop. This is bad practice and is observed in 4 functions already .For example the check
if(msg.sender != loan.lender) revert Unauthorized(); is made on every iteration of the array , while msg.sender is the same msg.sender that called the function. There is no point in that. This check can be placed before the for loop in the function logic.

The seizeLoan() function is executed in the same fashion - the whole function logic is executed on each iteration of the loop.My advise is to allow a user to seize one Loan at a time and be able to call the function as many times as he wants.

In order to not repeat myself I will just say that the refinance() function is implemented in the same way as the above mentioned functions. refinance() performs 6 transfers of tokens on every loop iteration.

To my understanding this is a medium but since this style of programming is implemented in 6 functions of this contract, which are basically the core functionality I am submitting it as high.Basically all functions that take arrays as inputs implement this logic.

Impact

Denial of Service

Tools Used

Manual Review

Recommendations

A simple fix might be to allow a user to borrow once per function call. In addition the code which executes on every iteration of the array should end after L236.

The same is applicable for repay() and giveLoan() and startAuction(). Declare the input checks before the for loop.In the for loop assign the elements of the arrays to the local variables and exit the loop.Proceed with the following logic outside of the loop.

In regards to the seizeLoan function I believe being able to seize one loan upon function call is more sustainable approach.The same is applicable and for refinance()