Beedle - Oracle free perpetual lending

Summary

When in Auction where buyLoan() occurs, the loans[loanId].lender will be updated with msg.sender value, based on the assumption that msg.sender is the pool.lender of the new pool which the loan is bought to. Since the buyLoan() function is callable by anyone, what happens if msg.sender is not the owner of the new pool which is provided in the parameter?

Vulnerability Details

In line 518, you can see that loan's lender is updated with the msg.sender. If this value doesn't equal to the new pool's lender, this will lead to 2 consequences:

1. The borrower can trigger this buyLoan() action with the matched config pool he/she doesn't own, so now the poolBalance of the new pool is deducted by the loan debt but the loan.lender is now the borrower. It's weird right, after this stage the borrower can avoid liquidation since only loan.lender can start the Dutch Auction in startAuction().

2. A malicious user can trigger this buyLoan() action with the matched config pool he/she doesn't own, so now the poolBalance of the new pool is deducted by the loan debt but the loan.lender is now the malicious user. At this stage, borrower's action of repay() and lender's action of seizeLoan() will be underflow and revert, which bricks the core functionality of a normal lending borrowing protocol. Because when the above actors doing those action, pools[poolId].outstandingLoans is 0 so it can't be deducted with the loan.debt.

In repay(): https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L303-L314

In seizeLoan(): https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L570-L575

When getting the poolId bytes32 poolId = getPoolId(loan.lender, loan.loanToken, loan.collateralToken), loan.lender is passed as the malicious user address but not the actual pool's lender which the loan was bought, so it returns a whole different poolId, explaining why pools[poolId].outstandingLoans is 0 which will revert the transaction when deduct the loan.debt as the malicious user doesn't have any pool under him/her.

POC

I will make a POC for case 1 (the borrower), the case 2 should be the same but there is another random user will call the buyLoan() to brick the repay() and seizeLoan() of the original borrower and lender.

Actors: borrower, lender1, lender2

1. Lender puts the loan on Auction by calling startAuction().

2. The random user sees a matched pool config and enough balance from the pool of lender2, he then calls buyLoan() with the loanId and poolId of lender2 as the parameters.

3. Lender1 is happy with his increased poolBalance, that's fine. But for the lender2, he doesn't know why his poolBalance is down and outstandingLoans is up, perhaps it won't be shown on the front-end UI as the loan.lender now is the borrower.

4. Borrower now can enjoy endless loan until he wants to repay for it, the lender2 can't start auction for the loan or give the loan to another pool as these 2 functions require the msg.sender has to be the loan.lender. This hurts the protocol operation as lender2 loan funds may stuck forever if the borrower won't repay the loan, also the lender2 can't seize the loan itself to get the collateral assets as I mentioned above.

Impact

1. The borrower of the loan can prevent his/her loan from being liquidated. Creating bad outstanding debts in the protocol.

2. The borrower and lender of the new pool will always be reverted when calling repay() or seizeLoan() if anyone passed the pool of that new lender since loan.lender is set under the one who called buyLoan().

Tools Used

Manual

Recommendations

Change the address of loans[loanId].lender to pools[poolId].lender, not the msg.sender. https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L518