fix(eth): Hardening ERC-20 detection logic
What the developer wrote
Authored by PrisionMike
fix(eth): Hardening ERC-20 detection logic
Previously, we were checking just the 'shape' and size of the contract to determine if it's an ERC-20 contract call. We now directly look at the function signature, if it is familiar. Changed function name for clarity as it handles non-ERC-20 contracts as well.
[no changelog]
What changed, and why it matters
This commit tightens how a Trezor hardware wallet decides whether an Ethereum transaction is a standard ERC-20 token transfer/approval. Previously the code guessed based on the transaction's overall shape and size; now it also checks the actual function signature bytes. The change removes an assumption that any matching transaction must be either a transfer or approval, which could have led the device to display a token-style confirmation for contract calls that only happened to look similar. There is no public claim that this was exploited, and the commit does not label itself as a security fix.
Treat as a defensive hardening patch. Review whether the previous size-only heuristic could have caused misleading UI labels for non-ERC-20 contract calls of the same data length, and consider whether a security advisory is warranted if user funds could have been at risk. No immediate emergency response is indicated by the diff alone.
Security signals we found
Hardening of smart-contract call classification
Removal of size-only heuristic for ERC-20 detection
Function selector now validated before parameter parsing
Assertion that forced transfer/approve-or-nothing classification removed
No changelog entry and no explicit security framing by vendor
Evidence from the diff
In core/src/apps/ethereum/sign_tx.py, the ERC-20 handling logic is renamed to _handle_known_contract_calls and refactored. The function now reads the 4-byte function selector before, not after, checking size/shape conditions, and only treats the call as ERC-20 transfer/approve if func_sig is SC_FUNC_SIG_TRANSFER or SC_FUNC_SIG_APPROVE. The previous code read the function selector inside the size branch and then still returned the selector to the caller even when it did not match a known ERC-20 function. The caller later asserted func_sig was either the transfer selector or None, but that assertion could be bypassed by a malformed or non-standard call that had the right size. The patch also removes that assertion, making the function usable for non-ERC-20 known contract calls in the future.
Changed components
core/src/apps/ethereum/sign_tx.pyEthereum transaction signing flowERC-20 token transfer/approve confirmation UIInspect captured patch +29 / −32
diff --git a/core/src/apps/ethereum/sign_tx.py b/core/src/apps/ethereum/sign_tx.py
index 45be9de4..9dd7de13 100644
--- a/core/src/apps/ethereum/sign_tx.py
+++ b/core/src/apps/ethereum/sign_tx.py
@@ -174,9 +174,8 @@ async def confirm_tx_data(
if await handle_staking(msg, defs.network, address_bytes, maximum_fee, fee_items):
return
- # Handle ERC-20 known functions
- token, token_address, func_sig, recipient, value = await _handle_erc20(
- msg, defs, address_bytes
+ token, token_address, func_sig, recipient, value = (
+ await _handle_known_contract_calls(msg, defs, address_bytes)
)
if token is tokens.UNKNOWN_TOKEN:
@@ -220,7 +219,6 @@ async def confirm_tx_data(
chunkify=bool(msg.chunkify),
)
else:
- assert func_sig == constants.SC_FUNC_SIG_TRANSFER or func_sig is None
assert value is not None
recipient_str = (
@@ -309,7 +307,7 @@ async def handle_staking(
return False
-async def _handle_erc20(
+async def _handle_known_contract_calls(
msg: MsgInSignTx,
definitions: Definitions,
address_bytes: bytes,
@@ -326,41 +324,40 @@ async def _handle_erc20(
token = None
token_address = None
- func_sig = None
recipient = address_bytes
value = int.from_bytes(msg.value, "big")
+
+ data_reader = BufferReader(data_initial_chunk)
+ if data_reader.remaining_count() < SC_FUNC_SIG_BYTES:
+ return token, token_address, None, recipient, value
+ func_sig = data_reader.read_memoryview(SC_FUNC_SIG_BYTES)
+
if (
len(msg.to) in (40, 42)
and len(msg.value) == 0
and msg.data_length == 68
and len(data_initial_chunk) == 68
+ and func_sig in (SC_FUNC_SIG_TRANSFER, SC_FUNC_SIG_APPROVE)
):
- data_reader = BufferReader(data_initial_chunk)
- if data_reader.remaining_count() < SC_FUNC_SIG_BYTES:
- return token, token_address, func_sig, recipient, value
-
- func_sig = data_reader.read_memoryview(SC_FUNC_SIG_BYTES)
- if func_sig in (SC_FUNC_SIG_TRANSFER, SC_FUNC_SIG_APPROVE):
- # The two functions happen to have the exact same parameters, so we treat them together.
- # This will need to be made into a more generic solution eventually.
-
- # arg0: address, Address, 20 bytes (left padded with zeroes)
- # arg1: value, uint256, 32 bytes
- if data_reader.remaining_count() < SC_ARGUMENT_BYTES * 2:
- return token, token_address, None, recipient, value
- arg0 = data_reader.read_memoryview(SC_ARGUMENT_BYTES)
- assert all(
- byte == 0
- for byte in arg0[: SC_ARGUMENT_BYTES - SC_ARGUMENT_ADDRESS_BYTES]
- )
- recipient = bytes(arg0[SC_ARGUMENT_BYTES - SC_ARGUMENT_ADDRESS_BYTES :])
- arg1 = data_reader.read_memoryview(SC_ARGUMENT_BYTES)
- if func_sig == SC_FUNC_SIG_APPROVE and all(byte == 255 for byte in arg1):
- # "Unlimited" approval (all bits set) is a special case
- # which we encode as value=None internally.
- value = None
- else:
- value = int.from_bytes(arg1, "big")
+ # The two functions happen to have the exact same parameters, so we treat them together.
+ # This will need to be made into a more generic solution eventually.
+ # arg0: address, Address, 20 bytes (left padded with zeroes)
+ # arg1: value, uint256, 32 bytes
+
+ if data_reader.remaining_count() < SC_ARGUMENT_BYTES * 2:
+ return token, token_address, None, recipient, value
+ arg0 = data_reader.read_memoryview(SC_ARGUMENT_BYTES)
+ assert all(
+ byte == 0 for byte in arg0[: SC_ARGUMENT_BYTES - SC_ARGUMENT_ADDRESS_BYTES]
+ )
+ recipient = bytes(arg0[SC_ARGUMENT_BYTES - SC_ARGUMENT_ADDRESS_BYTES :])
+ arg1 = data_reader.read_memoryview(SC_ARGUMENT_BYTES)
+ if func_sig == SC_FUNC_SIG_APPROVE and all(byte == 255 for byte in arg1):
+ # "Unlimited" approval (all bits set) is a special case
+ # which we encode as value=None internally.
+ value = None
+ else:
+ value = int.from_bytes(arg1, "big")
token = definitions.get_token(address_bytes)
token_address = address_bytes
Why this scored 63/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.