Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high

Unchecked ERC20 transfers can lead to funds draining/freezing

sonny2k

Summary

Some major tokens went live before ERC20 was finalised, resulting in a discrepancy whether the transfer functions a) should return a boolean or b) revert/fail on error. The current best practice is that they should revert, but return “true” on success. However, not every token claiming ERC20-compatibility is doing this — some only return true/false; some revert, but do not return anything on success. This is a well known issue, heavily discussed since mid-2018.

Vulnerability Details

Some tokens do not revert on failure, but instead return false (e.g. ZRX). tranfser/transferfrom is directly used to send tokens in many places in the contract and the return value is not checked.
If the token transfer fails, it will cause a lot of serious problems.

As document stated, the lender can freely specify the pool's loan/collateral tokens, so there are chances that these tokens are used in production.

Some examples of how much damage non-revert tokens on failure can cause in production:

In the addToPool function, if loan token is ZRX, the lender can increase the pool balance without providing any loan token and then drain other lenders' deposits of loan token via removeFromPool function.

_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
);

Also this can affect the removeFromPool function, if the transfer of loan token to the lender somehow fails, the lender's loan token is frozen forever in the contract since poolBalance has been decreased by the amount but the token is not transferred successfully to the lender.

_updatePoolBalance(poolId, pools[poolId].poolBalance - amount);
// transfer the loan tokens from the contract to the lender
IERC20(pools[poolId].loanToken).transfer(msg.sender, amount);

For borrower's cases, it can be exploited in the repay function, if loan token is ZRX, the borrower can repay the loan without providing any loan token.

// transfer the loan tokens from the borrower to the pool
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
);

And if the collateral token is ZRX, the borrower can borrow any amount of loan token without providing collateral assets.

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

Impact

If loan or collateral token used in the pool is not reverting on failures, it can be exploited to drain liquidity in the pool as well as blindly freeze assets of lender forever in the contract.

Tools Used

Manual

Recommendations

  1. Using OpenZeppelin’s SafeERC20 versions with the safeTransfer and safeTransferFrom functions that handle the return value check as well as non-standard-compliant tokens.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.