refactor(core/ethereum): simplify and rename `_send_request_chunk()`
What the developer wrote
Authored by Roman Zeyde
refactor(core/ethereum): simplify and rename `_send_request_chunk()`
Also, check `EthereumTxAck.data_chunk` size, similar to `apps.common.chunked.get_data_chunk()`.
[no changelog]
What changed, and why it matters
This commit refactors how a Trezor hardware wallet asks for pieces of an Ethereum transaction's data from the host computer. It renames an internal helper and, importantly, adds a size check so the host cannot return a chunk larger than the device requested. That oversized-chunk condition could previously lead to memory corruption or unexpected behavior during transaction signing.
Treat this as a security-hardening fix and include it in release notes. Review whether other message types that stream chunked data from the host perform equivalent length checks, and add regression tests for oversized EthereumTxAck.data_chunk responses.
Security signals we found
New length validation on attacker-controlled input (host-supplied data_chunk)
Prevention of oversized response chunks during streaming transaction signing
Alignment with existing defensive check in apps.common.chunked.get_data_chunk()
Refactor reduces exposed surface by returning bytes instead of full message object
Evidence from the diff
In core/src/apps/ethereum/sign_tx.py, _send_request_chunk() is renamed to _get_next_chunk() and now returns the raw data_chunk bytes instead of the full EthereumTxAck message. The new implementation requests up to _DATA_CHUNK_SIZE bytes, awaits the host’s EthereumTxAck, then validates that len(resp.data_chunk) <= req.data_length. If the host replies with more data than requested, a DataError(‘Too much data’) is raised. This mirrors the validation already present in apps.common.chunked.get_data_chunk().
Changed components
core/src/apps/ethereum/sign_tx.pyEthereum transaction signing flowHost-device chunked data exchange for EthereumTxRequest/EthereumTxAckInspect captured patch +9 / −7
diff --git a/core/src/apps/ethereum/sign_tx.py b/core/src/apps/ethereum/sign_tx.py
index 187e1f4d..bffd312b 100644
--- a/core/src/apps/ethereum/sign_tx.py
+++ b/core/src/apps/ethereum/sign_tx.py
@@ -18,7 +18,7 @@ if TYPE_CHECKING:
from buffer_types import AnyBytes
from typing import Sequence
- from trezor.messages import EthereumSignTx, EthereumTxAck
+ from trezor.messages import EthereumSignTx
from trezor.ui.layouts import StrPropertyType
from trezor.utils import HashWriter
@@ -147,8 +147,7 @@ async def request_initial_data(msg: MsgInSignTx, sha: HashWriter) -> AnyBytes:
while (
data_left > 0 and initial_data_length + _DATA_CHUNK_SIZE <= _MAX_DATA_STORED
):
- resp = await _send_request_chunk(data_left)
- chunk = resp.data_chunk
+ chunk = await _get_next_chunk(data_left)
initial_data[initial_data_length : initial_data_length + len(chunk)] = chunk
data_left -= len(chunk)
initial_data_length += len(chunk)
@@ -305,8 +304,7 @@ def _get_digest_length(msg: EthereumSignTx, data_total: int) -> int:
def create_data_chunk_loader(h: HashWriter) -> DataChunkLoader:
async def data_chunk_loader(data_left: int) -> AnyBytes:
- resp = await _send_request_chunk(data_left)
- chunk = resp.data_chunk
+ chunk = await _get_next_chunk(data_left)
h.extend(chunk)
return chunk
@@ -329,13 +327,17 @@ async def _confirm_data_chunks(
data_left -= len(chunk)
-async def _send_request_chunk(data_left: int) -> EthereumTxAck:
+async def _get_next_chunk(data_left: int) -> AnyBytes:
from trezor.messages import EthereumTxAck
from trezor.wire.context import call
req = EthereumTxRequest()
req.data_length = min(data_left, _DATA_CHUNK_SIZE)
- return await call(req, EthereumTxAck)
+ resp = await call(req, EthereumTxAck)
+ data_chunk = resp.data_chunk
+ if len(data_chunk) > req.data_length:
+ raise DataError("Too much data")
+ return data_chunk
def _sign_digest(
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.