Summary

ERC20 external call in the addToPool function of the Lender contract is vulnerable to re-entrancy attacks and unhandled return values for transfer and transferFrom functions. This vulnerability allows an attacker to manipulate the pool balance and cause losses to borrowers' collateral funds. The issue arises due to improper handling of the loanToken and collateralToken transfer operations.

Vulnerability Details

Untrusted pools leads to re-entrancy and poolbalance manipulation :
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L187
The addToPool function doesn't include any re-entrancy guard, allowing malicious contracts to potentially call this function repeatedly before it completes execution, leading to unexpected behaviour and manipulation of the poolBalance.
In detail, here the attacker will skip/erase transferFrom functionality in his own malicious contract and re-entered into function and manipulate PoolBalance.

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L267-L275
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L317-L332
here, also malformed transfer function skip transfering FTK to user (if he wants to, he it can gave him access to re-enter but no use with reentrancy here) and client/borrower's loss his collateral holdings and it may stuck inside the contract.
POC (borrow fun):
loadToken - ("FakeToken", FTK)
collateralToken - ("USD-Zebra", USDz)
debt = 20 FTK
collateral = 20 USDz
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L267-L275
the malicious-contract stored in loadToken, can skip the transfer, but borrower looses the collateral since he dont receive FTK.

Impact

1. pool balance will be shown very higher than originally it has.

2. borrower losses his collateral funds

Tools Used

Recommendations

loanToken address is always decided by the contract owner right in this case the loantoken.
use OpenZeppelin’s SafeERC20 wrapper functions and Re-entrancy guard.

bool result = IERC20(pools[poolId].loanToken).safeTransferFrom( msg.sender, address(this), pools[poolId].poolBalance + amount ); require(result,"Error"); // Update pool here after successfull transfer