Beedle - Oracle free perpetual lending

Summary

The refinance() function in Lender contract performs couple of different ERC20 transfers. The return values of those transfers are not checked. Some tokens do not revert on transfer failure, but return false instead. The lack of return value check may allow the malicious borrower to redeem collateral without paying back the debt.

Vulnerability Details

The refinance() function allows the borrower to transfer their loan to a different pool. Effectively the function closes the loan in the previous pool and opens a loan in the new pool. If the amount of debt/collateral does not match between the old and the new loan, the difference is transferred from the contract to the borrower, or from the borrower to the contract, to cover the mismatch.

The problem is the return values of transfer calls are not checked. This is specially problematic with the transfer of loanToken from the borrower to the contract, as the function never confirms if the loanToken transfer was really successful.

Please consider the following scenario:

1. Alice borrows 1 million TKN for 1 million USDC of collateral from the pool A. The TKN is a non-standard ERC20 token that does not revert on transfer failure, but returns false instead.

2. Alice immediately spends all of her TKNs and her current TKN balance is 0.

3. She calls the refinance() method, transfering her loan to the compatible pool B, specifying both the debt and collateral as zero in the Refinance object.

4. As the new debt is zero, the refinance function will attempt to transfer 1 million TKN from Alice to the protocol. This transfer will fail because of the insufficient balance, but the function exceution will proceed and the collateral will be transferred back to Alice.

5. Alice has stolen 1 million TKN from the protocol in one transaction.

The problematic unchecked transfer:
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L642-L646

Impact

The malicious borrower can redeem the collateral without paying back the debt. This requires the loanToken of the pool not to revert on failure.

The unsafe ERC20 operations have been reported in the known issues section under low category, however this particular exploitation of them is a separate, high-risk issue.

Tools Used

Manual Review

Recommendations

Replace the transfer/transferFrom calls with the OpenZeppelin safeTransfer/safeTransferFrom.