Beedle - Oracle free perpetual lending

Summary

The refinance() function can be called by the borrower to refinance a loan under new lending condition. The function moves the loan partially or totally from one lender or pool to another. The function however does not restrict the refinance to the exact same pool, same terms, as the current loan. This means that, when the lender starts an auction, the borrower can refinance the loan to the same pool, without changes, just to stop the auction. This effectively means that the borrower can keep the loan going, unilaterally, for a very long time.

Vulnerability Details

Refinance is allowed during an auction, and it will move the loan to another pool. If the loan.auctionStartTimestamp is not the max uint256 it means that an auction has started. When refinancing the auctionStartTimestamp is set to the system default of max uint256.

The following POC, on Lender.t.sol, proves that a refinance to the same pool is allowed, resetting the auctionStartTimestamp to the system default to stop the ongoing auction.

function test_auctionCanceledByRefinance() public {
    test_borrow();

    vm.startPrank(lender1);

    uint256[] memory loanIds = new uint256[](1);
    loanIds[0] = 0;

    lender.startAuction(loanIds);

    (address oldLender,,,,,,,,uint256 startTime,) = lender.loans(0);

    // auction is going
    assertEq(startTime, block.timestamp);

    // borrower refinances to the same pool
    vm.startPrank(borrower);
    Refinance memory r = Refinance({
        loanId: 0,
        poolId: keccak256(
            abi.encode(
                address(lender1),
                address(loanToken),
                address(collateralToken)
            )
        ),
        debt: 100*10**18,
        collateral: 100*10**18
    });
    Refinance[] memory rs = new Refinance[](1);
    rs[0] = r;

    lender.refinance(rs);

    address newLender;
    (newLender,,,,,,,, startTime,) = lender.loans(0);

    // Loan auction was stoped by a refinance in the same pool
    assertEq(startTime, type(uint256).max);
    // After the refinance the lender is the same
    assertEq(oldLender, newLender);
}

Impact

The borrower can keep the loan going for a very long time by stopping every auction attempt of the lender.

Tools Used

Foundry

Recommendations

Refinancing should only be allowed to a new pool. This is confirmed by the beedle devs README.md. In a refinance, the borrower is able to move their loan to a new pool under new lending conditions.

We should add a check that the pool to move the loan is not the same as the old one.

diff --git a/src/Lender.sol b/src/Lender.sol

@@ -609,6 +666,8 @@ contract Lender is Ownable {

            // get the pool info
            Pool memory pool = pools[poolId];
+            // refinancing to the same pool not allowed.
+            if (loan.lender == pool.lender) revert Unauthorized();
            // validate the new loan
            if (pool.loanToken != loan.loanToken) revert TokenMismatch();
            if (pool.collateralToken != loan.collateralToken)