Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`Rescuable::_safe_transfer_from` uses low level `call` method to transfer tokens, can fail in certain scenarios.

tdey

Summary

Rescuable::_safe_transfer_from uses low level call method to transfer tokens

Vulnerability Details & Impact

As per the documentation the supported tokens are:

  • ETH

  • WETH

  • ERC20 (any token that follows the ERC20 standard)

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();
}
}

Rescuable::_safe_transfer_from uses low level call method to transfer tokens, it can lead to issues like:

  1. Using low-level call for ERC20 transfers can be vulnerable. It does handles the different implementations of transferFromcorrectly.

  2. It only checks if the call was successful, and not if the transfer itself was successful. Some ERC20 tokens return boolean and some tokens don't. E.g. USDT doesn't return a boolean, in case the call is successful but transfer fails there is no way to identify that.

  3. No checks for 0 address present. Can make tokens permanently stuck in case 0 address is entered.

link to code

Tools Used

Manual review

Recommendations

Use OpenZeppelin's safeTransferFrommethod, which accounts for all different scenarios.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.