Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

 `_safe_transfer` and `_safe_transfer_from` Functions Are Not Safe for All ERC20 Tokens

aresaudits

Summary

Vulnerability Details

The `_safe_transfer and _safe_transfer_from` functions are intended to safely transfer tokens. However, these functions do not handle the case where some ERC20 tokens do not return a boolean value upon a successful transfer. According to the ERC20 standard, some tokens may not return true on a successful transfer, and instead, they may just not revert. The current implementation assumes that a successful call must return true, which can lead to unnecessary transaction reverts for such tokens.

/**
* @dev Safe transfer.
* @param token The token to transfer. If 0, it is ether.
* @param to The address of the account to transfer to.
* @param amount The amount to transfer.
*/
function _safe_transfer(
address token,
address to,
uint256 amount
) internal {
(bool success, ) = token.call(
abi.encodeWithSelector(TRANSFER_SELECTOR, to, amount)
);
​
if (!success) {
revert TransferFailed();
}
}
​
/**
* @dev Safe transfer.
* @param token The token to transfer. If 0, it is ether.
* @param from The address of the account to transfer from.
* @param to The address of the account to transfer to.
* @param amount The amount to transfer.
*/
function _safe_transfer_from(
address token,
address from,
address to,
uint256 amount
) internal {
(bool success, ) = token.call(
abi.encodeWithSelector(TRANSFER_FROM_SELECTOR, from, to, amount)
);
​
if (!success) {
revert TransferFailed();
}
}
​

Impact

The current implementation may revert transactions for ERC20 tokens that do not return a boolean value upon a successful transfer. This can lead to failed transactions and a poor user experience when interacting with such tokens.

Tools Used

Recommendations

use safeERC20.sol

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-weird-erc-20-return-boolean-Rescuable

I believe the issues and duplicates do not warrant low severity severity as even if the call to transfers returns false instead of reverting, there is no impact as it is arguably correct given there will be insufficient funds to perform a rescue/withdrawal. This will not affect `tillIn()` as there are explicit balance [checks that revert accordingly](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L255-L260) to prevent allowing creation of offers without posting the necessary collateral

Support

FAQs

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