Making sure I understand how SafeERC20 works

:computer: Environment

  • @openzeppelin/contracts@3.0.1

:memo:Details

This post is about best practices on how to handle the missing return value bug, a well-known vulnerability that exists in hundreds of ERC20 tokens that don’t return a value in their transfer and transferFrom functions. The knock-on effect of this bug is that users may end up with their tokens locked up in some contracts, just like it happened with the BNB liquidity pool in Uniswap.

After reading the OpenZeppelin Erc20 README, I understand the following things:

  1. ERC20.sol is the core implementation, which correctly adheres to the Erc20 standard
  2. SafeERC20.sol is an extension to the core implementation that fends off against other non-compliant tokens like BNB or OMG

It is this “fends off” that I want to shed some light on with this post. Are the following statement correct?

  1. A contract that extends only from ERC20.sol puts its users’ funds at risk, should they ever deposit vulnerable tokens.
  2. A contract that extends from both the core ERC20.sol and uses the SafeERC20.sol library makes it possible to both deposit and withdraw vulnerable tokens.
3 Likes

Hi @PaulRBerg,

I was a bit confused whether you were referring to interacting with a token or creating a token. I assume that you are referring to a contract interacting with ERC20 tokens.

I suggest reading the reply from @frangio in SafeERC20, TokenTimelock, wrappers - #2 by frangio

SafeERC20 is not an ERC20 extension that you use to make your token safe (OpenZeppelin’s ERC20 is already safe :slightly_smiling_face:). It’s a helper to make safe the interaction with someone else’s ERC20 token, in your contracts.

The documentation (https://docs.openzeppelin.com/contracts/3.x/api/token/erc20) generated from README.adoc includes the following:

There a few core contracts that implement the behavior specified in the EIP:

  • IERC20: the interface all ERC20 implementations should conform to.
  • ERC20: the implementation of the ERC20 interface, including the name, symbol and decimals optional standard extension to the base interface.

Finally, there are some utilities to interact with ERC20 contracts in various ways.

  • SafeERC20 is a wrapper around the interface that eliminates the need to handle boolean return values.
  • TokenTimelock can hold tokens for a beneficiary until a specified time.

The SafeERC20 API documentation has the following:

Wrappers around ERC20 operations that throw on failure (when the token contract returns false). Tokens that return no value (and instead revert or throw on failure) are also supported, non-reverting calls are assumed to be successful. To use this library you can add a using SafeERC20 for ERC20; statement to your contract, which allows you to call the safe operations as token.safeTransfer(…​) , etc.


Answering your questions:

SafeERC20.sol is a utility to use when interacting with ERC20 contracts.

A contract that interacts with tokens using either IERC20.sol or ERC20.sol with a token that doesn't match IERC20 return values has potential for the tokens to be locked inside the contract.

A contract that interacts with tokens using SafeERC20.sol can interact with a token that doesn't match IERC20 return values without risking the the tokens get locked inside the contract.

3 Likes

Excellent! Thanks for confirming all my asumptions.

You're right, I was referring to a contract that interacts with Erc20 tokens. In my mind, I had the use case of a wrapper Erc20 contract, which transfers in and out other Erc20 tokens, so maybe that's why my formulation was somewhat ambiguous.

1 Like

Hello @abcoathup Sorry but a litle question :

About this Attack “Overflow”
https://consensys.github.io/smart-contract-best-practices/known_attacks/#integer-overflow-and-underflow

On ERC20.sol Openzeppelin not checking if balance[recipient] += amount is over uint256 size No ?

Or i dont understand where is the checking ?

EDIT : Or the new version of Solidity check alone the overflow ?

Thanks

1 Like

Hey, I think you mean the compiler version is 0.8.x, cause from solidity 0.8, overflow checks are built into the compiler, for more details, you can read about this article:

1 Like

I kinda wonder if I should use safeERC20’s token.safeTransfer(to, value) vs. simply require(token.transfer(to, value), “Token transfer error”)? If I understand it correctly, the difference is that, safeTransfer will assume the transfer is success if there’s no return value, while a required transfer will revert if there’s no return value, both seem to prevent the non-compliant tokens from being locked in the contract itself?

No, the vanilla require(token.transfer(to, value), "Error message") will not fend non-compliant tokens. Non-compliant means that no boolean flag is returned. Execution would revert immediately.