Beedle - Oracle free perpetual lending

Summary

The Lender.sol contract includes multiple transfer and transferFrom functions that are critical vulnerabilities since they do not validate the return statements of the ERC20 tokens between the lender and borrower, which means that a transaction could potentially not go through and the contract state could still be updated.

In the mentioned contract, the unchecked return of transfer and transferFrom are critical vulnerabilities that could in this particular instance, lead to funds getting stuck in pools, borrowers erasing debts without repaying, e.g., essentially draining the pool funds.

Vulnerability Details

The improper invocation of transfer and transferFrom are used across the Lender.sol. Across these contracts transfer and transferFrom are being called without checking the return statement. According to the current standard, the caller cannot assume that a falsy bool value or lack of value is never returned. The critical risk is that the code would be executed and update the contracts’ state variables while, in actuality, the transfer and transferFrom have not gone through.

Although, these vulnerabilities have been disclosed as low (the assumption being that they simply do not follow the standard). The following vulnerability details show that these vulnerabilities should instead be considered high as they include:

• Funds getting stuck in pools.

• Borrowers can erase debt without repaying, e.g., essentially draining the pool funds.

Impact

Below I will list the different functions where they appear, together with a short explanation of what they could cause. Lastly, I will walk through one of the (many) critical vulnerabilities that appear by using transfer and transferFrom in the current contract.

In the following explanations, assume that a token that does not follow the ERC20-standard is used, such as USDT (which does not return a bool), and that the transfer and transferFrom functions do not go through.

Example: If the borrower borrows a token, it is assumed that it is USDT. If the lender lends a token it is assumed that the token is USDT.

Lender.sol

setPool - If the lender is setting a new higher poolBalance a borrower risk not being able to borrow from the pool even though it “should” have funds. If the lender is setting a lower poolBalance, the funds risk getting stuck in the pool (since the lender cannot withdraw them).

addToPool - the pools will have the wrong higher value shown, barring borrowers from borrowing the “added” funds.

removeFromPool - The pools will have the wrong lower updated value shown, essentially blocking the lender from withdrawing funds (funds are stuck).

borrow - The borrower will be unable to borrow funds, but the poolBalance will decrease, and his debt will increase. Essentially getting rugged of his collateral tokens.

repay - See lengthy explanation below.

giveLoan - The feeReceiver will not receive the protocolInterest

buyLoan - The feeReceiver will not receive the protocolInterest.

seizeLoan - The collateralToken would not be transferred to the lender. And since we delete loans[loanId], the loan will not ever be able to be transferred to the lender.

refinance - We can change the debt and collateral of the refinancing loan without adding or removing more loan/collateral tokens, causing a mismatch in the loan’s actual worth.

Repay

The borrower can erase their own and everyone else’s debt by pretending to repay the loan if the loan token doesn’t conform to the ERC20-standard and get back their collateral tokens. The following POC shows the steps:

1. The borrower takes a loan from a pool that lends out USDT and takes WETH as collateral.

2. The borrower will take this loan and send it to another EOA or smart contract.

3. The borrower then calls the repay function with their loanId.

4. When the contract tries to execute the code:

It will not succeed since the borrower does not have the USDT they lent. However, since this does not revert, the execution of the code will simply continue, and the contract will pay back the collateral tokens to the borrower before deleting the loan completely (delete loans[loanId]).

1. The borrower can repeat these steps to drain the pool completely.

Tools Used

Manual Review.

Recommendations

The recommendation is to replace the transfer and transferFrom to use Open Zeppelin’s safeTransfer and safeTransferFrom, which accounts for tokens not following the ERC20-token standard and will revert in accordance.