Beedle - Oracle free perpetual lending

Summary

When loan in the auction any Pool is capable of buying it then in buyLoan function of Lender.sol anyone can pass any other lender's poolId to buy loan from it.There is no check implemented to check that the lender of the pool corresponding to that passed poolId is calling this. So Attacker can becomes lender in that loan and loan.debt will be deducted from that lender's pool of that passed poolId, in which attacker is not lender. By becoming lender in the loan without paying that loan to borrower. He will exploit this(details below in Exploitation) and transfer that loan collateral will be transferred to attacker's account without paying loan. And loss of loan.debt + lenderInterest amount of loanToken tokens will be for that lender whose pool's poolId is passed to buyLoan by attacker.

Vulnerability Details

Vulnerable Code : buyLoan Function

At line 518 caller of the buyLoan function is becoming the lender in loan in that auction. There is no check implemented to prevent him from not using any other lender's poolId. So attacker will exploit it, becomes lender and exploit this to get loan.debt + lenderInterest loan tokens or loan.collateral - govFee amount of collateral tokens into his account.

Exploitation Steps :

If loan is in auction and any Pool is capable of buying it. By passing that loan's loanId and pool's poolId to buyLoan function

1. Attacker will become lender of that loan in buyLoan function as stated above.
   Now attacker is the lender but not have a pool, so pools[poolId].outstandingLoans will be 0 and whenever someone tries to repay the loan or attacker want to auction loan no one can buy it or attacker want to seize loan it will revert in all because of underflow error in repay and seizeLoan at line

   pools[poolId].outstandingLoans -= loan.debt;

   or in , buyLoan, and giveLoan functions at line

   pools[oldPoolId].outstandingLoans -= loan.debt;

   So to avoid this error

2. Attacker will create a pool using setPool function. And in parameters in pool creation pass loanToken and collateralToken same as of loan in which he became lender maliciously. Pass poolBalance of loanToken tokens equal to loan. debt so it can be borrowed again by attacker from his own pool and outStandingLoans on that pool equals to loan.debt. Pass maxLoanRatio very high more than loan.debt*10e18 so that he can pass check in borrow function

   uint256 loanRatio = (debt * 10 ** 18) / collateral;

   if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();

   when he himself borrows from it's own pool by just passing collateral just 1 wei of collateralToken.

3. Now by calling borrow function, he will borrow the loan from his own pool by passing collateral just 1 wei of collateralToken, take loan of all his pool's poolBalance since which is equals to loan.debt So outstandingLoansof his pool will be equals toloan.debt.
   Note:

4. After that he will start auction by calling startAuction function for the loan he became lender maliciously. If someone buys from it good he will get loan.debt + lenderInterest amount of loanToken tokens.

5. If auction ended without bought he will call seizeLoan function for the loan he became lender maliciously and can get loan.collateral - govFee amount of collateral tokens into his account.

6. He will keep his own loan permanently in protocol as he himself withdrawal his poolBalance as a loan from it, now that pool only have 1 wei of collateralToken, it is nothing to attacker and keep that loan stay there forever.

Note:

1. Do above steps 2 and 3 in same txn so no one else can benefit from attacker's pool.

2. loan.debt used above is the debt parameter value of that loan in which attacker became lender maliciously. same as for loan.collateral use above.

3. 3. His own loan means what attacker took from his own pool to make his pool's outStandingLoans equals to loan.debt to avoid underflow.

Impact

If loan is in auction and any Pool is capable of buying it.
Attacker can become lender in that loan passing loanId of loan and poolId of that pool.
It will be loss of loan.debt + lenderInterest amount of loanToken tokens for lender of that poolId pool. These tokens will be deducted from lender's pools[poolId].poolBalance while this pool didn't become lender in that loan.
After becoming lender attacker can exploit this loan and can get loan.debt + lenderInterest amount of loanToken tokens OR If not bought in auction then he can seize the loan and get loan.collateral amount of collateralToken tokens.

Tools Used

Manual Review

Recommendations

Write a if condition which can check that whatever poolId is passed into the buyLoan function parameters, the caller of buyLoan function ie. msg.sender is also the lender of pool corresponding to that poolId. So that any random person can't pass that poolId to buy loan to it by cutting it's poolBalance. By this we can assure only lender of that poolId will call this function and buy the loan from it's poolBalance.

Recommended Changes in buyLoan Function of Lender.sol file.

Note: Here we only write relevant part of buyLoan function to show where the recommended code should be added. Full code of that function can be accessed here