20,000 USDC
View results
Submission Details
Severity: high
Valid

Non-standard ERC20 tokens opens the contract to vulnerabilities and loss of funds.

Summary

The protocol allows users to choose what ERC20 tokens to set up their pools with and do not limit what tokens are allowed.

In the current implementation the Lender.sol contract is open to vulnerabilities involving ERC20 tokens that do not revert when a transaction fails and tokens that have a fee-on-transfer.

Vulnerability Details

ERC20 tokens that do not revert when transfer or transferFrom fails:

Here a malicious lender can call setPool with a high poolBalance and not approve the Lender.sol contract to spend their ERC20 loanToken. If the malicious user is setting their loan token to one that does not revert on transfer fail (examples are ZRX, USDT) the following line of code would silently fail:

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

The malicious lender would then have successfully created a pool with whatever balance they specified without sending the funds to the Lender.sol contract. That lender would then be able to call the removeFromPool function and steal the funds being held in the contract for other legitimate pools.

ERC20 tokens that have a fee-on-transfer:

Some tokens implement a fee-on-transfer mechanism (examples are STA, PAXG), meaning that the actual value transferred won’t be amount but it will actually be amount - fee. This can create many problems in Lender.sol as it assumes the amount received or transferred when using transfer or transferFrom is always the full amount specified.

Impact

This vulnerability has been listed as high as the likelihood of it happening is high since users can choose any ERC20 tokens they want to set up their pools and the impact can be users losing their funds.

Tools Used

Manual Review.

Recommendations

Use OpenZeppelin’s SafeERC20 library and its safe methods for ERC20 transfers. For fee-on-transfer tokens, check the balance before and after transfer and transferFrom and use the difference between the two as the actual transferred value.

I also recommend the protocol to take a look at the following link to ensure that the protocol is ready to handle all ERC20s.

https://github.com/d-xo/weird-erc20

Support

FAQs

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