MorpheusAI

Summary

ERC20 functions may not behave as expected.

For example: return values are not always meaningful.

Among other issues, if the approve, transfer, or transferFrom fails, it will not be known immediately.

Vulnerability Details

The ERC20 standard does not require approve and other functions to return a boolean indicating success.

ERC20 tokens do not have a standardized return value for the transfer and transferFrom functions.

Some implementations might not return a value, while others could return false on failure instead of reverting the transaction. This can lead to unexpected behavior if the return value is not properly checked.

In the provided code, the direct call to approve on the IERC20 interface does not check the return value:

Found in contracts/L1Sender.sol Line: 69

transferFrom example -
Found in contracts/mock/GatewayRouterMock.sol Line: 15

There are other instances of these (check github attachments above).

Previous similar findings in other contests:

• code4rena - Fiat Dao - [H-01] Unsafe usage of ERC20 transfer and transferFrom

• Sherlock - Telcoin - Issue M-2: Unsafe ERC20 methods

• Halborn - Portal Gate - (HAL-03) USE OF UNSAFE APPROVE
  METHOD IN ZAPINETH FUNCTION

Impact

This is not ideal for unsafe ERC20 approve,transfer,transferFrom that do not handle non-standard erc20 behavior.

The contract will malfunction for certain tokens. Resulting in silent failures, that will cause the entire contract or parts of the contract to break/ not behave as intended.

This might even result in financial loss.

Tools Used

Manual Review, Analyzer, AI

Recommendations

OpenZeppelin's SafeERC20 library wraps ERC20 functions and ensures that they behave as expected, regardless of the token implementation. It reverts the transaction if the token contract does not return true (or if it does not return any value, which is interpreted as false).

To mitigate potential issues with non-compliant ERC20 tokens, it is recommended to use SafeERC20 for interactions with ERC20 tokens. Here's how you would use SafeERC20 to safely call approve, transfer and transferFrom:

1. First, import the library:

2. Then include this line inside the contract code:

3. Now, make changes to all these lines, across 4 contracts:

approve > safeIncreaseAllowance() or safeDecreaseAllowance(), as per need.

Change in contracts/L1Sender.sol Line: 69

Change in contracts/L1Sender.sol Line: 76

Change in contracts/L1Sender.sol Line: 91

Change in contracts/L1Sender.sol Line: 95

NOTE : We are not switching approve with safeApprove because, safeApprove is deprecated.