Result of transfer / transferFrom not checked.

Severity

Medium Risk

Summary

The ERC20.transfer() and ERC20.transferFrom() functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead.

Vulnerability Details

Some tokens (like USDT) don't correctly implement the EIP20 standard and their transfer/ transferFrom function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert.

Impact

Tokens that don't actually perform the transfer and return false are still counted as a correct transfer and tokens that don't correctly implement the latest EIP20 spec, like USDT, will be unusable in the protocol as they revert the transaction because of the missing return value.

Code snippet

Fees.sol
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Fees.sol#L43C8-L43C24
```
IERC20(WETH).transfer(staking, IERC20(WETH).balanceOf(address(this)));
```

Lender.sol
https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L152C13-L156C15


IERC20(p.loanToken).transferFrom(
            p.lender,
            address(this),
            p.poolBalance - currentBalance
);

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L159C11-L162C15
```solidity
 IERC20(p.loanToken).transfer(
                p.lender,
                currentBalance - p.poolBalance
            );

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L187C8-L191C11

 IERC20(pools[poolId].loanToken).transferFrom(
            msg.sender,
            address(this),
            amount
        );

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L203C9-L203C70

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

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L267C12-L275C15

 IERC20(loan.loanToken).transfer(feeReceiver, fees);
            // transfer the loan tokens from the pool to the borrower
            IERC20(loan.loanToken).transfer(msg.sender, debt - fees);
            // transfer the collateral tokens from the borrower to the contract
            IERC20(loan.collateralToken).transferFrom(
                msg.sender,
                address(this),
                collateral
            );

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L317C12-L332C15

 IERC20(loan.loanToken).transferFrom(
                msg.sender,
                address(this),
                loan.debt + lenderInterest
            );
            // transfer the protocol fee to the fee receiver
            IERC20(loan.loanToken).transferFrom(
                msg.sender,
                feeReceiver,
                protocolInterest
            );
            // transfer the collateral tokens from the contract to the borrower
            IERC20(loan.collateralToken).transfer(
                loan.borrower,
                loan.collateral
            );

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L403C12-L403C76

 IERC20(loan.loanToken).transfer(feeReceiver, protocolInterest);

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L505C9-L505C72

IERC20(loan.loanToken).transfer(feeReceiver, protocolInterest);

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L563C12-L568C15

 IERC20(loan.collateralToken).transfer(feeReceiver, govFee);
            // transfer the collateral tokens from the contract to the lender
            IERC20(loan.collateralToken).transfer(
                loan.lender,
                loan.collateral - govFee
            );

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L642C17-L646C19

IERC20(loan.loanToken).transferFrom(
                    msg.sender,
                    address(this),
                    debtToPay - debt
                );

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L651C16-L653C85

 IERC20(loan.loanToken).transfer(feeReceiver, fee);
                // transfer the loan tokens from the contract to the borrower
                IERC20(loan.loanToken).transfer(msg.sender, debt - debtToPay - fee);

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L656C13-L656C76

IERC20(loan.loanToken).transfer(feeReceiver, protocolInterest);

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L663C17-L667C19

IERC20(loan.collateralToken).transferFrom(
                    msg.sender,
                    address(this),
                    collateral - loan.collateral
                );

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L670C17-L673C19

IERC20(loan.collateralToken).transfer(
                    msg.sender,
                    loan.collateral - collateral
                );

Staking.sol

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Staking.sol#L39C9-L39C62

TKN.transferFrom(msg.sender, address(this), _amount);

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Staking.sol#L49C9-L49C43

TKN.transfer(msg.sender, _amount);

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Staking.sol#L55C9-L55C58

WETH.transfer(msg.sender, claimable[msg.sender]);

Tools Used

Manual Review

Recommendations

Recommend using OpenZeppelin's SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.

Cyfrin / 2023-07-beedle