fix(core): fix eth signing `data_length` check
What the developer wrote
Authored by M1nd3r
fix(core): fix eth signing `data_length` check
[no changelog]
(cherry picked from commit 3f4f146340b7adefb7ebf149414fd39b7a4efb00)
What changed, and why it matters
This commit replaces internal programming assertions (which crash the device if they fail) with proper error handling for Ethereum transaction signing when a payment request is involved. It ensures that if someone supplies a payment request verifier, the transaction must have zero data bytes and must actually include a payment request object. Without the fix, a malformed or unexpected input could trigger an assertion failure instead of a clean error, potentially causing the device to crash or behave unpredictably during signing.
Treat as a low-to-moderate hardening fix. Review whether the payment request verifier path can be reached with attacker-controlled msg fields, and confirm that DataError is handled safely by the surrounding message loop. No immediate emergency response is indicated, but firmware users should update when convenient.
Security signals we found
assert replaced with explicit exception
input validation added for payment request path
Ethereum transaction signing hardening
potential denial-of-service via malformed message mitigated
Evidence from the diff
In core/src/apps/ethereum/sign_tx.py, the function confirm_tx_data previously used assert data_length == 0 and assert msg.payment_req is not None when payment_request_verifier was non-None. Assertions can be optimized away or can crash the firmware. The patch converts these into explicit DataError exceptions and adds a check that msg.payment_req is not None. This is a hardening change: it turns an internal invariant into a validated precondition with a user-facing error, reducing the risk of a device crash or undefined behavior on unexpected host messages.
Changed components
core/src/apps/ethereum/sign_tx.pyEthereum transaction signing flowpayment request verifier handlingInspect captured patch +9 / −3
diff --git a/core/src/apps/ethereum/sign_tx.py b/core/src/apps/ethereum/sign_tx.py
index 15c9e98a..0a05ddeb 100644
--- a/core/src/apps/ethereum/sign_tx.py
+++ b/core/src/apps/ethereum/sign_tx.py
@@ -290,10 +290,16 @@ async def confirm_tx_data(
)
if payment_request_verifier is not None:
- assert data_length == 0
+ if data_length != 0:
+ raise DataError(
+ "Data length must be 0 when `payment_request_verifier` is provided."
+ )
+
+ if msg.payment_req is None:
+ raise DataError(
+ "Payment request (`payment_req`) must not be None when `payment_request_verifier` is provided."
+ )
- # If a payment_request_verifier is provided, then msg.payment_req must have been set.
- assert msg.payment_req is not None
assert recipient_str is not None
payment_request_verifier.add_output(value, recipient_str or "")
Why this scored 49/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.