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

ERC20 unchecked return values of transfer, approve and transferFrom

cryptomoon

Summary

ERC20 unchecked return values of transfer, approve and transferFrom

Vulnerability Details

The transfer(), transferFrom() and approve() function returns a boolean value indicating success. This parameter needs to be checked to see if the transfer or approval has been successful. Some tokens like EURS and BAT will not revert if the transfer failed but return false instead. Tokens that don't actually perform the transfer and return false are still counted as a correct transfer.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/utils/Rescuable.sol#L84-L117

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 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 protocol would still create offer or change the state even if transfer or approval wasn't successful.

Tools Used

Manual review

Recommendations

Use OZ’s SafeERC20’s safeTransferFrom(), safeTransfer and safeApprove function.

Updates

Lead Judging Commences

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