chore(core/ethereum): reject invalid initial chunk
What the developer wrote
Authored by Roman Zeyde
chore(core/ethereum): reject invalid initial chunk
[no changelog]
What changed, and why it matters
This commit fixes a validation bug in Trezor's Ethereum transaction signing. Previously, the device only checked whether the initial data chunk was too large when the transaction also contained non-zero data length. Now it always rejects an oversized initial chunk, even when the declared data length is zero. The change is defensive and closes a path where malformed input could slip past validation.
No urgent action beyond applying the patch. Users on affected firmware versions should update when available. Developers should review whether other conditional validation checks have similar branch-dependent gaps.
Security signals we found
Input validation bypass fixed
Defensive length check broadened
Test coverage added for zero-data-length case
Evidence from the diff
In core/src/apps/ethereum/sign_tx.py, the check len(msg.data_initial_chunk) > data_length was moved outside the if data_length > 0: branch. Previously, if data_length was 0, the size check was skipped entirely, so a message could supply a non-empty data_initial_chunk without raising DataError. The patch ensures the check runs for all transactions. The test is updated to cover both non-zero and zero declared data lengths.
Changed components
core/src/apps/ethereum/sign_tx.pyEthereum transaction signing flowInspect captured patch +28 / −25
diff --git a/core/src/apps/ethereum/sign_tx.py b/core/src/apps/ethereum/sign_tx.py
index e0592331..6f4545f5 100644
--- a/core/src/apps/ethereum/sign_tx.py
+++ b/core/src/apps/ethereum/sign_tx.py
@@ -357,8 +357,9 @@ def check_common_fields(msg: MsgInSignTx) -> None:
# prevent exceeding the limit we use a stricter limit on data length.
if data_length > 16_000_000:
raise DataError("Data length exceeds limit")
- if len(msg.data_initial_chunk) > data_length:
- raise DataError("Invalid size of initial chunk")
+
+ if len(msg.data_initial_chunk) > data_length:
+ raise DataError("Invalid size of initial chunk")
if len(msg.to) not in (0, 40, 42):
raise DataError("Invalid recipient address")
diff --git a/tests/device_tests/ethereum/test_signtx.py b/tests/device_tests/ethereum/test_signtx.py
index eeed386c..a60cd192 100644
--- a/tests/device_tests/ethereum/test_signtx.py
+++ b/tests/device_tests/ethereum/test_signtx.py
@@ -591,30 +591,32 @@ def test_signtx_data_pagination(session: Session, scroll: bool, size: int):
def test_signtx_data_bad_init(session: Session):
- DATA = b"A" * 256
-
- with session.test_ctx as client:
-
- def _filter(msg: MessageType) -> MessageType:
- req = messages.EthereumSignTx.ensure_isinstance(msg)
- assert req.data_initial_chunk is not None
- req.data_initial_chunk += b"EXTRA"
- return req
+ def _sign_calldata(data: bytes) -> None:
+ with session.test_ctx as client:
+
+ def _filter(msg: MessageType) -> MessageType:
+ req = messages.EthereumSignTx.ensure_isinstance(msg)
+ assert req.data_initial_chunk is not None
+ req.data_initial_chunk += b"EXTRA"
+ return req
+
+ client.set_filter(message_type=messages.EthereumSignTx, callback=_filter)
+ with pytest.raises(TrezorFailure, match="Invalid size of initial chunk"):
+ ethereum.sign_tx(
+ session,
+ n=parse_path("m/44h/60h/0h/0/0"),
+ nonce=0x0,
+ gas_price=0x14,
+ gas_limit=0x14,
+ to="0x1d1c328764a41bda0492b66baa30c4a339ff85ef",
+ chain_id=1,
+ value=0xA,
+ tx_type=None,
+ data=data,
+ )
- client.set_filter(message_type=messages.EthereumSignTx, callback=_filter)
- with pytest.raises(TrezorFailure, match="Invalid size of initial chunk"):
- ethereum.sign_tx(
- session,
- n=parse_path("m/44h/60h/0h/0/0"),
- nonce=0x0,
- gas_price=0x14,
- gas_limit=0x14,
- to="0x1d1c328764a41bda0492b66baa30c4a339ff85ef",
- chain_id=1,
- value=0xA,
- tx_type=None,
- data=DATA,
- )
+ _sign_calldata(b"A" * 256)
+ _sign_calldata(b"")
def test_signtx_data_bad_ack(session: Session):
Why this scored 47/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.