Audit Application Form
Let's talk
DeFi patterns: ERC20 token transfers Howto
Author: Sergey Boogerwooger, Dmitry Zakharov
Security researchers at MixBytes
Security researchers at MixBytes
Intro
ERC20 token transfers in DeFi protocols can be tricky. Despite being a well-known procedure token transfer bugs still exist. To mitigate serious security issues, it's essential to consider various scenarios related to token transfers in your DeFi project. This article aims to highlight these problems, provide references to real bugs, demonstrating these problems, and outline possible options to consider when implementing token transfers in your protocols.
Token transfers
Each DeFi protocol requires token transfers. There are several ways to achieve this. Here are some common patterns:
Each of these patterns has specific security concerns. One common issue that spans across all token transfer cases is the possibility of using attacker-provided tokens. Many DeFi protocols allow users or pool/market creators to provide arbitrary ERC20 tokens, which can lead to security risks. Maliciously crafted tokens may have "poisoned" transfer*(), balanceOf() functions, fees-on-transfer, blacklisting, and reentrancy abilities.
- Direct token transfer: Directly call token.transfer(...), sending tokens to an external user/contract.
- Token transfer with approval: Transfer tokens from a user to the protocol with token.transferFrom() (which requires a preliminary token.approve() from the user).
- Token callbacks: Utilize tokens (such as ERC-677, ERC-777, etc.) or protocol callbacks (like token.transferAndCall() or this "classic" example from Uinswap V3 here) and calculate the resulting contract balance change.
Each of these patterns has specific security concerns. One common issue that spans across all token transfer cases is the possibility of using attacker-provided tokens. Many DeFi protocols allow users or pool/market creators to provide arbitrary ERC20 tokens, which can lead to security risks. Maliciously crafted tokens may have "poisoned" transfer*(), balanceOf() functions, fees-on-transfer, blacklisting, and reentrancy abilities.
Direct transfer
When transferring tokens via transfer(), several security concerns must be addressed.
First and foremost, the transfer status must be checked. This case is described in many places, while the most known is the safeTransfer() implementation by OpenZeppelin, whose core function can be found here.
ERC20 standard dictates the obligatory return of a success/failure value. However, non-standard tokens, such as USDT/BNB/etc, may not return a value at all (We don't include really "sick" tokens, i.e., returning false on successful transfer). To handle this, the "safe" transfer return value check can be described as: "the return value is optional (in rare cases), but if data is returned, it must not be false".
It is crucial to correctly handle unsuccessful transfers in your protocol. The demonstration bug is described here.
Direct transfer from the protocol means that protocol sends tokens to some external address (user or contract), so, each function containing transfer() is a natural "way out" for a possible attacker and must be maximally protected.
This includes a very desirable reentrancy protection. Even when using safeTransfer() correctly and following the Check-Effects-Interactions pattern, in some cases such functions can still be exploited (for example, the ones with "cross-contract" or "read-only" reentrancy). Special attention should be paid to ERC-677 and ERC-777 tokens with transferAndCall() callbacks, receive hooks, and any other possibilities for calling additional code "inside" the transfer. An example of a bug where transfer() with reentrancy is used is here.
Additionally, consider the following potential issues:
First and foremost, the transfer status must be checked. This case is described in many places, while the most known is the safeTransfer() implementation by OpenZeppelin, whose core function can be found here.
ERC20 standard dictates the obligatory return of a success/failure value. However, non-standard tokens, such as USDT/BNB/etc, may not return a value at all (We don't include really "sick" tokens, i.e., returning false on successful transfer). To handle this, the "safe" transfer return value check can be described as: "the return value is optional (in rare cases), but if data is returned, it must not be false".
It is crucial to correctly handle unsuccessful transfers in your protocol. The demonstration bug is described here.
Direct transfer from the protocol means that protocol sends tokens to some external address (user or contract), so, each function containing transfer() is a natural "way out" for a possible attacker and must be maximally protected.
This includes a very desirable reentrancy protection. Even when using safeTransfer() correctly and following the Check-Effects-Interactions pattern, in some cases such functions can still be exploited (for example, the ones with "cross-contract" or "read-only" reentrancy). Special attention should be paid to ERC-677 and ERC-777 tokens with transferAndCall() callbacks, receive hooks, and any other possibilities for calling additional code "inside" the transfer. An example of a bug where transfer() with reentrancy is used is here.
Additionally, consider the following potential issues:
- Transferring zero tokens: in some cases, it can lead to problems, which will be discussed in the "approve" section.
- Controllable revert: If an attacker can cause the protocol to always (or controllably) revert, it can lead to problems. A good example, demonstrating both problems is here.
approve() + transferFrom()
This variant is the most widely used method for receiving tokens from a user as there is no universal method to notify a contract about receiving the tokens. The common way to receive 100 tokens from a user is to make this user first approve 100 tokens, and then call the target function in the protocols (which will perform transferFrom() ). The same mechanism applies when taking multiple tokens from a user. Furthermore, this method can be used to transfer tokens to another contract by giving the allowance to an external contract and then calling the target function within it.
What are the dangerous scenarios with this kind of token transfers?
First of all - allowance amounts. A user can set a very large allowance amount to allow the protocol to continuously use tokens from their account. This means that if the protocol is somehow (due to a hack or error) able to execute transferFrom(), it can drain a large number of tokens from the user's address.
[NOTE] It includes attacks not only on smart contracts but also on the UI of the project. Even an XSS vulnerability, which is usually not so critical in traditional Web2, can cause a very serious security incident in Web3. A good example of an attack on UI is the Badger DAO hack.
As we previously mentioned, non-standard ERC20 tokens can bring problems to the protocol, and a good example of such behaviour is the USDT token. USDT has a non-standard approve() function, which cannot modify the approval amount unless approve(0) is called first. This means that if the protocol doesn't spend all the allowance, it cannot use USDT tokens if there is no possibility to reduce the allowance to zero. So, keep this in mind if you plan to use USDT and allowances in your protocol. A bug demostrating this behaviour is here.
Allowances themselves don't have a lot of direct security issues, but can be a critical part of an exploit as the "last step" of the attack. Therefore, the functions that use approve() or transferFrom() are well-protected and thoroughly covered with tests.
What are the dangerous scenarios with this kind of token transfers?
First of all - allowance amounts. A user can set a very large allowance amount to allow the protocol to continuously use tokens from their account. This means that if the protocol is somehow (due to a hack or error) able to execute transferFrom(), it can drain a large number of tokens from the user's address.
[NOTE] It includes attacks not only on smart contracts but also on the UI of the project. Even an XSS vulnerability, which is usually not so critical in traditional Web2, can cause a very serious security incident in Web3. A good example of an attack on UI is the Badger DAO hack.
As we previously mentioned, non-standard ERC20 tokens can bring problems to the protocol, and a good example of such behaviour is the USDT token. USDT has a non-standard approve() function, which cannot modify the approval amount unless approve(0) is called first. This means that if the protocol doesn't spend all the allowance, it cannot use USDT tokens if there is no possibility to reduce the allowance to zero. So, keep this in mind if you plan to use USDT and allowances in your protocol. A bug demostrating this behaviour is here.
Allowances themselves don't have a lot of direct security issues, but can be a critical part of an exploit as the "last step" of the attack. Therefore, the functions that use approve() or transferFrom() are well-protected and thoroughly covered with tests.
Transfers via callbacks
This method of interaction is extremely popular. In essence, it means "do what you want in your callback, but change my contract's balance from X to Y; I will check it after your callback is executed". This approach requires that the caller is a contract or the calldata for the external callback function is passed as a parameter. Since callbacks can implement almost any code, there is no common classification of possible bugs in this context. However, some crucial considerations must be mentioned.
The execution "inside" the token transfer is a natural form of "reentrancy", allowing an attacker to call other parts of the protocol while the callback is still being executed. The final amounts of tokens transferred are not updated yet, and the protocol "doesn't know" about the token transfer. In some cases, this can lead to problems. An example of such a bug can be found here.
As with any external call, the callback should be provided with sufficient gas. Furthermore, if you want your users to have the ability to interact with your protocol using an external trading contract, the function calling the callback should have enough gas remaining to ensure a successful execution.
The execution "inside" the token transfer is a natural form of "reentrancy", allowing an attacker to call other parts of the protocol while the callback is still being executed. The final amounts of tokens transferred are not updated yet, and the protocol "doesn't know" about the token transfer. In some cases, this can lead to problems. An example of such a bug can be found here.
As with any external call, the callback should be provided with sufficient gas. Furthermore, if you want your users to have the ability to interact with your protocol using an external trading contract, the function calling the callback should have enough gas remaining to ensure a successful execution.
Amounts inconsistency
This type of bug is extremely common when transferring tokens. The calculation and usage of token amounts can differ significantly from one protocol to another, resulting in a wide range of potential bugs. Many cases related to token transfers involve amounts passed to these functions and used before and after transfers. Let's discuss them.
You cannot pass a certain amount to the transfer() or transferFrom() function and be sure that this amount will be actually transferred. For instance:
You cannot pass a certain amount to the transfer() or transferFrom() function and be sure that this amount will be actually transferred. For instance:
token.transferFrom(from, address(this), amount);
// ...
mint(from, amount); // non-trusted amountThis approach will not work with tokens that have a "fee-on-transfer" mechanism (such as USDT, which can enable transfer fees at any moment) or with tokens that have extended logic on transfer. A notable example of such a bug can be found here.
Another problem to consider is that same amount can be processed differently by the transferFrom() function (in the token) and in the mint() function (in the protocol). For example, in cases with complicated logic inside the mint() function. Although this is not a direct issue related to token transfers, it's essential to remember that token amounts and amounts used in the protocol can have different precision and be used differently in calculations. An example of how processing amounts differently led to a zero result in one function and a non-zero result in another function is demonstrated here.
In cases where token transfers occur between two addresses controllable by the user, you should always consider the scenario where the sender and receiver are the same address. When combined with memory/storage caching issues, this can lead to bugs. The example snippet:
Another problem to consider is that same amount can be processed differently by the transferFrom() function (in the token) and in the mint() function (in the protocol). For example, in cases with complicated logic inside the mint() function. Although this is not a direct issue related to token transfers, it's essential to remember that token amounts and amounts used in the protocol can have different precision and be used differently in calculations. An example of how processing amounts differently led to a zero result in one function and a non-zero result in another function is demonstrated here.
In cases where token transfers occur between two addresses controllable by the user, you should always consider the scenario where the sender and receiver are the same address. When combined with memory/storage caching issues, this can lead to bugs. The example snippet:
uint256 fromBalance = _balances[from];
uint256 toBalance = _balances[to];
[.. snip ..]
_balances[from] = fromBalance - value;
_balances[to] = toBalance + value; // VULNERABLE!!!looks like safe, but in case when from equals to leads to the caching problem with toBalance. This bug is described here.
Operation with the values used in token transfers always includes considering their decimals and normalization. If your protocol uses multiple tokens, you should be extremely careful with decimals when saving and using values for/from token operations. An example when amounts with different precision is demonstrated here.
We will describe many calculation issues in our next articles, many of them not related only to token transfers, now let's proceed to the section "how it should be made".
Operation with the values used in token transfers always includes considering their decimals and normalization. If your protocol uses multiple tokens, you should be extremely careful with decimals when saving and using values for/from token operations. An example when amounts with different precision is demonstrated here.
We will describe many calculation issues in our next articles, many of them not related only to token transfers, now let's proceed to the section "how it should be made".
How it should be made
When implementing token transfers in your protocol, it's essential to carefully study and consider these points:
Now, let's take a look at code examples demonstrating (in our opinion) correct token transfer procedures, which fully demonstrate the principles above.
- Check the scenarios with "poisoned" tokens, subverted transfer(), transferFrom(), balanceOf() and other ERC20 functions. Restrict the set of ERC20 tokens allowed for your protocol, or beware of your users and utilize all possible defenses against malicious tokens. Avoid the usage of weird tokens (useful list on GitHub).
- Check the status of transfer. Consider the possibility of usage of non-standard tokens, like USDT (not returning resulting value), "fee-on-transfer" tokens, blacklisting, and other "good" features of ERC20 tokens that can break your protocol. Consider the possibility of reentrancy from tokens containing hooks on transfers (ERC-677, ERC-777, etc.).
- If your protocol uses a callback for receiving tokens, ensure that protocol is safe against reentrancy from the callback.
- Use safeTransfer() and safeTransferFrom() functions for token transfers, always check and handle the result of transfer operations. Protect functions transferring tokens from reentrancy. Consider the possibility of cross-contract reentrancy.
- Check which contract address will be a receiver for users' allowances, estimate the risks of this contract being compromised.
- If there are multiple sending/receiving addresses controllable by user, check the scenario when the sender and receiver addresses are equal.
- If some functions use multiple tokens and somehow receive addresses of the contracts of these tokens, consider the scenario with the same token contract address repeated twice.
- When working with amounts of tokens with different precision, always normalize all amounts to the highest precision to avoid rounding errors.
- When saving and using amounts used in transfers, always consider the possibility of wrong usage of memory/storage values and caching.
- When writing tests involving token amounts, check the cases with extra small (e.g., 0,1,…) amounts and extra-large (e.g., type(uint256).max ) values if it makes sense.
Now, let's take a look at code examples demonstrating (in our opinion) correct token transfer procedures, which fully demonstrate the principles above.
Example: transfer()
The first example is related to direct transfer, and we'll take Aave V3 contracts, which use numerous safety features for token transfers. We'll discuss the executeBorrow() function, which sends underlying tokens to the borrower (we'll skip some parts, the full function is available here).
function executeBorrow(
// ...
// params.amount contain the target tokens amount
// ...
DataTypes.ExecuteBorrowParams memory params
// ...
) public {
DataTypes.ReserveData storage reserve = reservesData[params.asset];
// ...
ValidationLogic.validateBorrow(
// ...
DataTypes.ValidateBorrowParams({
// ...
asset: params.asset,
userAddress: params.onBehalfOf,
amount: params.amount,
// ...
})
);
// ...
if (params.releaseUnderlying) {
IAToken(reserveCache.aTokenAddress).transferUnderlyingTo(params.user, params.amount);
// transferUnderlyingTo() function is simply:
// IERC20(_underlyingAsset).safeTransfer(target, amount);
}
}
emit Borrow(
//...
);
}Let's proceed with our checklist above:
1st case: "Poisoned" tokens: The list of underlying ERC20 tokens is curated by Aave DAO and contains only "good" tokens, so this case is not applicable.
2nd case: Non-standard tokens: The safeTransfer() function from OpenZeppelinis used, which correctly checks the transfer status. Although blacklisting and "fee-on-transfer" are still potential issues for tokens like USDT, market tends to ignore them :). The function doesn't have external protection from reentrancy from ERC-677 and ERC-777 tokens, but since the transfer() of tokens is the last action in the function and all state changes are made before, so it seems to be okay.
3rd case: described above
4th, 5th case: not applicable (no transferFrom() )
6th, 7th: not applicable (no multiple sender/receipient addresses and loops with multiple token addresses)
8th, 9th cases: let's check the operations with params.amount, passed to the safeTransfer() function. amount (passed from the user) is validated by the validateBorrow() function. It's used in the calculation of totalDebt here:
1st case: "Poisoned" tokens: The list of underlying ERC20 tokens is curated by Aave DAO and contains only "good" tokens, so this case is not applicable.
2nd case: Non-standard tokens: The safeTransfer() function from OpenZeppelinis used, which correctly checks the transfer status. Although blacklisting and "fee-on-transfer" are still potential issues for tokens like USDT, market tends to ignore them :). The function doesn't have external protection from reentrancy from ERC-677 and ERC-777 tokens, but since the transfer() of tokens is the last action in the function and all state changes are made before, so it seems to be okay.
3rd case: described above
4th, 5th case: not applicable (no transferFrom() )
6th, 7th: not applicable (no multiple sender/receipient addresses and loops with multiple token addresses)
8th, 9th cases: let's check the operations with params.amount, passed to the safeTransfer() function. amount (passed from the user) is validated by the validateBorrow() function. It's used in the calculation of totalDebt here:
vars.totalDebt =
params.reserveCache.currTotalStableDebt +
vars.totalSupplyVariableDebt +
params.amount;where we need to make sure that all the values used in the calculation are consistent with amount, avoiding precision errors and problems with decimals across different assets. Checks of totalDebt and params.amount (here and here) should be examined to establish params.amount as a trusted value for token transfers.
10th case: You can verify the test coverage of all functions, focusing on token amounts in Aave tests by yourself.
10th case: You can verify the test coverage of all functions, focusing on token amounts in Aave tests by yourself.
Example: transferFrom()
The next example demonstrates the use of transferFrom() in the Lido protocol to transfer stETH tokens from the user to the protocol. Code (source here):
function _requestWithdrawal(uint256 _amountOfStETH, address _owner) internal returns (uint256 requestId) {
STETH.transferFrom(msg.sender, address(this), _amountOfStETH);
uint256 amountOfShares = STETH.getSharesByPooledEth(_amountOfStETH);
requestId = _enqueue(uint128(_amountOfStETH), uint128(amountOfShares), _owner);
_emitTransfer(address(0), _owner, requestId);
}1st, 2nd, 3rd cases: not applicable. The stETH token is a conservative, fully ERC20-compliant token, which eliminates the risks of "poisoned" tokens, tokens with transfer hooks, and fees on transfer. In the event of an unsuccessful transfer, it will simply revert, and there is no possibility of reentrancy from stETH. In addition, the withdrawal logic in Lido only puts the withdrawal request in the queue. Even though transferFrom() is called before the state changes (which is not typically recommended to avoid reentrancy risks),in this particular case, it's safe.
4th case: Since this function uses its own checked transferFrom(), compliant with the ERC20 standard, there is no need to use a "safe" implementation of transferFrom. The case with reentrancy was already described above.
5th case: Lido doesnt' use a specialized vault to store user's funds or allowances, so in the event of a compromise, funds aren't safe. However, Lido has a limited number of functions controlling transfers, and in the case of a compromise, this risk "stacks" with other risks, which are mitigated by many audits of its codebase.
6th and 7th cases: Not applicable, as there are no multiple sender/recipient addresses and loops over multiple tokens.
8th case: stETH uses the same decimals, and all operations with stETH and wstETH work with the same precision. As a result, all token transfer amounts in calculations are not affected by precision problems. While this does not guarantee that all math is automatically correct, the internal calculations in the protocol, which are not trivial in Lido, are outside the scope of this article.
9th case: Not applicable, as transfer amounts are not used in multi-stage computations here.
10th case: You can check the test coverage of all functions, by testing token amounts in Lido tests by yourself.
4th case: Since this function uses its own checked transferFrom(), compliant with the ERC20 standard, there is no need to use a "safe" implementation of transferFrom. The case with reentrancy was already described above.
5th case: Lido doesnt' use a specialized vault to store user's funds or allowances, so in the event of a compromise, funds aren't safe. However, Lido has a limited number of functions controlling transfers, and in the case of a compromise, this risk "stacks" with other risks, which are mitigated by many audits of its codebase.
6th and 7th cases: Not applicable, as there are no multiple sender/recipient addresses and loops over multiple tokens.
8th case: stETH uses the same decimals, and all operations with stETH and wstETH work with the same precision. As a result, all token transfer amounts in calculations are not affected by precision problems. While this does not guarantee that all math is automatically correct, the internal calculations in the protocol, which are not trivial in Lido, are outside the scope of this article.
9th case: Not applicable, as transfer amounts are not used in multi-stage computations here.
10th case: You can check the test coverage of all functions, by testing token amounts in Lido tests by yourself.
Example: callback
The next example is possibly the most well-known - it's the Uniswap's swap() function (V3). After calculating the target amounts, the final transfers are presented here. The code snippet:
(amount0, amount1) = zeroForOne == exactInput
? (amountSpecified - state.amountSpecifiedRemaining, state.amountCalculated)
: (state.amountCalculated, amountSpecified - state.amountSpecifiedRemaining);
// do the transfers and collect payment
if (zeroForOne) {
if (amount1 < 0) TransferHelper.safeTransfer(token1, recipient, uint256(-amount1));
uint256 balance0Before = balance0();
IUniswapV3SwapCallback(msg.sender).uniswapV3SwapCallback(amount0, amount1, data);
require(balance0Before.add(uint256(amount0)) <= balance0(), 'IIA');
} else {
if (amount0 < 0) TransferHelper.safeTransfer(token0, recipient, uint256(-amount0));
uint256 balance1Before = balance1();
IUniswapV3SwapCallback(msg.sender).uniswapV3SwapCallback(amount0, amount1, data);
require(balance1Before.add(uint256(amount1)) <= balance1(), 'IIA');
}In the two branches, the uniswapV3SwapCallback() function is called from the contract, initiating the swap. This callback can perform any actions, but the one of them must be a change to the pool's balance according to the calculated values. Notably, Uniswap doesn't check the status of transfers; instead, it relies solely on its own balance change. This approach is the only reliable way to work with arbitrary tokens, as it involves checking the final effect after all operations, which can include anything.
Let's proceed with our checklist.
1st, 2nd cases: Not applicable. Uniswap is a permissionless protocol, any tokens can be used. The user is responsible for performing the transfer, so all checks are on the caller's side.
3rd case: The swap() function is protected from the reentrancy by locking/unlocking the slots participating in the swap. In addition, all pool state modifications are made before the token transfers.
4th, 5th, 6th and 7th cases: Not applicable.
8th case: Precision of used amounts. Uniswap stores the reserves of used tokens in uint128 values, and no combinations of both reserves are used for calculating amounts. This case is not a concern.
9th case: Risks are on the client contracts' side.
10th case: You can verify the test coverage by checking the tests yourself.
Let's proceed with our checklist.
1st, 2nd cases: Not applicable. Uniswap is a permissionless protocol, any tokens can be used. The user is responsible for performing the transfer, so all checks are on the caller's side.
3rd case: The swap() function is protected from the reentrancy by locking/unlocking the slots participating in the swap. In addition, all pool state modifications are made before the token transfers.
4th, 5th, 6th and 7th cases: Not applicable.
8th case: Precision of used amounts. Uniswap stores the reserves of used tokens in uint128 values, and no combinations of both reserves are used for calculating amounts. This case is not a concern.
9th case: Risks are on the client contracts' side.
10th case: You can verify the test coverage by checking the tests yourself.
Conlculsion
As previously mentioned, token transfers can be tricky, and their widespread use in DeFi protocols doesn’t make them any less challenging. Unsafe token operations frequently show up in audit reports, especially when non-standard ERC20 tokens are involved. For this reason, it's essential to perform all the checks outlined above wherever ERC20 transfers are implemented in the code. Stay safe!
Who is MixBytes?
MixBytes is a team of expert blockchain auditors and security researchers specializing in providing comprehensive smart contract audits and technical advisory services for EVM-compatible and Substrate-based projects. Join us on Twitter to stay up-to-date with the latest industry trends and insights.
Disclaimer
The information contained in this Website is for educational and informational purposes only and shall not be understood or construed as financial or investment advice.
Other posts