Beedle - Oracle free perpetual lending

Summary

context: lender.sol

Some ERC20 tokens return a bool value to indicate whether a transaction failed or succeeded. Alice, a lender, creates a
pool with a loanToken that returns a bool value when the transfer function is called inside the loanToken.

Alice can use the lender.addToPoolBalance() function to add funds to the protocol and update her poolBalance state.
Additionally, she can use lender.removeToPoolBalance() to reduce funds from her poolBalance state, and then the protocol
transfers funds from the protocol to Alice.

Vulnerability Details

The vulnerability lies in lender.addToPoolBalance(),lender.setPool() and the lender takes an advantage from
lender.removeToPoolBalance().

 function addToPool(bytes32 poolId, uint256 amount) external {
    if (pools[poolId].lender != msg.sender) revert Unauthorized();
    if (amount == 0) revert PoolConfig();
    _updatePoolBalance(poolId, pools[poolId].poolBalance + amount);
    // transfer the loan tokens from the lender to the contract
    IERC20(pools[poolId].loanToken).transferFrom(
        msg.sender,
        address(this),
        amount
    );
}

 function removeFromPool(bytes32 poolId, uint256 amount) external {
    if (pools[poolId].lender != msg.sender) revert Unauthorized();
    if (amount == 0) revert PoolConfig();
    _updatePoolBalance(poolId, pools[poolId].poolBalance - amount);
    // transfer the loan tokens from the contract to the lender
    IERC20(pools[poolId].loanToken).transfer(msg.sender, amount);
}

 uint256 currentBalance = pools[poolId].poolBalance;

    if (p.poolBalance > currentBalance) {
        // if new balance > current balance then transfer the difference from the lender
        IERC20(p.loanToken).transferFrom(
            p.lender,
            address(this),
            p.poolBalance - currentBalance
        );
    } else if (p.poolBalance < currentBalance) {
        // if new balance < current balance then transfer the difference back to the lender
        IERC20(p.loanToken).transfer(
            p.lender,
            currentBalance - p.poolBalance
    }   

Let's take an example:

1. Alice calls the lender.addToPoolBalance() function without approving the protocol to transfer funds from her account.
   Consequently, the transferFrom function returns false, but Alice's poolBalance state is updated.

2. Later, Alice calls lender.removeFromPool() to reduce funds from her poolBalance, and the protocol will transfer the
   reduced balance to Alice's account. However, Alice did not send any funds to the protocol, leading to a loss of funds
   for the protocol.

Impact

This issue leads to a loss of funds for the protocol. Borrowers can also perform the same attack while providing
collateral tokens.

IERC20(loan.collateralToken).transferFrom(
            msg.sender,
            address(this),
            collateral
        );
        loans.push(loan);
        emit Borrowed(
            msg.sender,
            pool.lender,
            loans.length - 1,
            debt,
            collateral,
            pool.interestRate,
            block.timestamp
        );  

Tools Used

Manual

Recommendations

To mitigate this vulnerability, it is recommended to check the returned values from transfer and transferFrom functions
or use OpenZeppelin's safeTransfer and safeTransferFrom functions.

 function addToPool(bytes32 poolId, uint256 amount) external {

    if (pools[poolId].lender != msg.sender) revert Unauthorized();

    if (amount == 0) revert PoolConfig();

    _updatePoolBalance(poolId, pools[poolId].poolBalance + amount);

    // transfer the loan tokens from the lender to the contract

    (bool success) = IERC20(pools[poolId].loanToken).transferFrom(

                     msg.sender,

                     address(this),

                     amount

                     );

    require(success,"transaction failed");

}

 function removeFromPool(bytes32 poolId, uint256 amount) external {

    if (pools[poolId].lender != msg.sender) revert Unauthorized();

    if (amount == 0) revert PoolConfig();

    _updatePoolBalance(poolId, pools[poolId].poolBalance - amount);

    // transfer the loan tokens from the contract to the lender

    (bool success) = IERC20(pools[poolId].loanToken).transfer(msg.sender, amount);

    require(success,"transaction failed");

 }

By fixing this, the protocol can securely transfer funds based on the return values of transfer and transferFrom
functions.