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

Lack of access control in `buyLoan` function allows a malicious user to force another pool to pay off a borrower's loan, while also cheating them out of any future interest from the loan

toshii

Summary

When a user calls buyLoan, they specify a specific loanId and poolId. poolId can specify any pool which has an interestRate less than the currentAuctionRate (msg.sender does not need to be the lender for this pool). The logic of this contract first has the pool specified by poolId pay off the current debt owed by the user. The intention is to then update the loan specified by loanId to indicate that the borrower of that loan now needs to pay future interest to that pool. However, instead, the logic specifies that the the borrower needs to pay off that debt/interest to whatever pool is owned by msg.sender. Since there is no check that enforces that the pool specified by poolId is owned by msg.sender, this means msg.sender can effectively cheat the owner of poolId out of funds.

Vulnerability Details

Let's walk through the main logic of the buyLoan function, which takes in a loanId (loan to refinance) and poolId (pool to refinance into). When a pool whose interestRate is less than the currentAuctionRate is specified, there's the following logic:

...
if (pools[poolId].interestRate > currentAuctionRate) revert RateTooHigh();
// calculate the interest
(uint256 lenderInterest, uint256 protocolInterest) = _calculateInterest(
loan // @ from loanId
);
​
// reject if the pool is not big enough
uint256 totalDebt = loan.debt + lenderInterest + protocolInterest;
if (pools[poolId].poolBalance < totalDebt) revert PoolTooSmall();
​
// if they do have a big enough pool then transfer from their pool
_updatePoolBalance(poolId, pools[poolId].poolBalance - totalDebt);
pools[poolId].outstandingLoans += totalDebt;
...

With the _updatePoolBalance call, the specified poolId pool is required to pay down the debt of the user of the loanId loan.

Then, later in this function call, the loan is updated with the intention to have the borrower then pay off the future interest payments + debt to the pool which was used to refinance the loan:

...
loans[loanId].lender = msg.sender; // @ this is the issue
loans[loanId].interestRate = pools[poolId].interestRate;
loans[loanId].startTimestamp = block.timestamp;
loans[loanId].auctionStartTimestamp = type(uint256).max;
loans[loanId].debt = totalDebt;
...

Instead of that, the loan specifies the pool owned by msg.sender to be the recipient of the future interest payments + debt. This means that the actual owner of the poolId pool will be cheated out of funds.

Impact

The caller of buyLoan can force another pool to pay off a user's debt, while also ensuring that they will never get paid back, meaning potentially significant losses for those pool owners.

Tools Used

Manual review

Recommendations

Either the lender for the loan should be updated as follows in the buyLoan function:

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

or there should be a check that msg.sender is the lender for the specified pool.

Support

FAQs

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