Public commit record
40/100 · Thin
What the developer wrote
Authored by Ioan Bizău
fix: raise exceptions
[no changelog]
✓ Subject identifies a change✓ Uses a recognizable type or scope! No meaningful explanatory body
What changed, and why it matters
This commit fixes two places in Trezor's payment-request handling where error objects were created but never actually thrown (missing 'raise'). One missing raise meant a payment request with memos but no nonce would be accepted instead of rejected; the other meant an unknown memo type would be silently ignored instead of rejected. The patch adds the missing 'raise' keywords, refactors nonce handling to support unit testing, and adds tests to confirm the errors are now raised. It is a genuine bug fix, but the practical security impact is bounded because the verifier still checks the cryptographic signature before any payment is approved.
Recommended action
Treat as a low-to-moderate security hardening fix. Review whether any released firmware shipped with the missing raises and assess if nonce-less memo-bearing payment requests could have been exploited in practice. No immediate emergency response appears warranted because signature verification still gates final approval, but the fix should be included in the next release and changelog.
Security signals we found
01
Missing 'raise' caused intended security checks to be silently skipped
02
Nonce validation bypass could allow memo-bearing payment requests without a nonce
03
Unknown memo type would be ignored instead of rejected
04
Fix is accompanied by regression tests for both error paths
05
Debug-only code paths are clearly gated by __debug__ and unit-test context detection
Evidence from the diff
In core/src/apps/common/payment_request.py, two DataError exceptions were instantiated without being raised: ‘DataError(“Missing nonce in payment request.”)’ when payment_request.nonce is absent but memos exist, and ‘DataError(“Unrecognized memo type in payment request.”)’ for unsupported memo types. The patch prefixes both with ‘raise’. It also extracts nonce cache access into methods (get_expected_nonce/delete_nonce_cache) and provides debug overrides so unit tests can exercise the production nonce path without a wire context. Test vectors and signatures are updated to include the debug nonce, and new tests verify the now-raised errors in both debug and production-nonce modes.
Changed components
core/src/apps/common/payment_request.pyPaymentRequestVerifier classPayment request nonce validationPayment request memo validationInspect captured patch +124 / −11
diff --git a/core/src/apps/common/payment_request.py b/core/src/apps/common/payment_request.py
index 0645d04e2..4e219544b 100644
--- a/core/src/apps/common/payment_request.py
+++ b/core/src/apps/common/payment_request.py
@@ -59,15 +59,35 @@ class PaymentRequestVerifier:
if not _is_coin_swap(payment_request):
raise DataError("Only COIN SWAP payment requests are supported.")
+ def get_expected_nonce(self) -> bytes | None:
+ from storage.cache_common import APP_COMMON_NONCE
+
+ return context.cache_get(APP_COMMON_NONCE)
+
+ def delete_nonce_cache(self) -> None:
+ from storage.cache_common import APP_COMMON_NONCE
+
+ context.cache_delete(APP_COMMON_NONCE)
+
if __debug__:
def _use_debug_key(self) -> None:
# nist256p1 public key of m/0h for "all all ... all" seed.
+ # Corresponding private key: b"\x05\x62\x35\xb0\x47\x6f\x05\x7f\x27\x65\x21\x97\x24\xf7\xf1\x80\x7d\x58\x80\x2b\x55\x0e\xd5\xbf\x6f\x73\x05\x0a\xf5\x45\x63\x00"
+ # keeping it here for reference in case tests need to be updated!
self.PUBLIC_KEY = b"\x03\xd9\xd9\x3f\x89\xc6\x96\x3b\x94\xbb\xd7\xa5\x11\x88\x28\xe4\x4c\x1c\x39\x59\x15\xac\xe8\x48\x88\x71\x7f\x56\x8c\xb0\x19\x74\xc3"
def _use_debug_verification(self) -> None:
self.verify_payment_request_is_supported = lambda payment_request: None
+ def _use_debug_nonce_verification(self) -> None:
+ self.get_expected_nonce = lambda: b"DEBUG NONCE"
+
+ def _del() -> None:
+ self.get_expected_nonce = lambda: None
+
+ self.delete_nonce_cache = _del
+
def __init__(
self,
payment_request: PaymentRequest,
@@ -77,7 +97,6 @@ class PaymentRequestVerifier:
8, 32
] = 8, # amount is normally 8 bytes, but for EVM assets it is 32 bytes
) -> None:
- from storage.cache_common import APP_COMMON_NONCE
from trezor.crypto.hashlib import sha256
from trezor.utils import HashWriter
@@ -88,6 +107,11 @@ class PaymentRequestVerifier:
if __debug__:
self._use_debug_key()
self._use_debug_verification()
+ if context.CURRENT_CONTEXT is None:
+ # in unit tests we don't have a context, so we replace
+ # the nonce verification with the debug version
+ # otherwise (device tests, etc) we should use the proper nonce verification
+ self._use_debug_nonce_verification()
payment_request = _sanitize_payment_request(payment_request)
self.verify_payment_request_is_supported(payment_request)
@@ -107,13 +131,13 @@ class PaymentRequestVerifier:
if payment_request.nonce:
nonce = bytes(payment_request.nonce)
- if context.cache_get(APP_COMMON_NONCE) != nonce:
+ if self.get_expected_nonce() != nonce:
raise DataError("Invalid nonce in payment request.")
- context.cache_delete(APP_COMMON_NONCE)
+ self.delete_nonce_cache()
else:
nonce = b""
if payment_request.memos:
- DataError("Missing nonce in payment request.")
+ raise DataError("Missing nonce in payment request.")
writers.write_bytes_fixed(self.h_pr, b"SL\x00\x24", 4)
writers.write_bytes_prefixed(self.h_pr, nonce)
@@ -151,7 +175,7 @@ class PaymentRequestVerifier:
writers.write_bytes_prefixed(self.h_pr, memo.title.encode())
writers.write_bytes_prefixed(self.h_pr, memo.text.encode())
else:
- DataError("Unrecognized memo type in payment request.")
+ raise DataError("Unrecognized memo type in payment request.")
writers.write_uint32_le(self.h_pr, slip44_id)
diff --git a/core/tests/test_apps.common.payment_request.py b/core/tests/test_apps.common.payment_request.py
index 5a096ceb0..c305d3caf 100644
--- a/core/tests/test_apps.common.payment_request.py
+++ b/core/tests/test_apps.common.payment_request.py
@@ -45,6 +45,21 @@ def patch_prod(func: Callable[P, None]) -> Callable[P, None]:
return wrapper
+# Decorator that replaces DEBUG version of nonce verification
+# by the PROD version (which use wire context)
+def patch_prod_nonce(func: Callable[P, None]) -> Callable[P, None]:
+
+ def wrapper(*args: P.args, **kwargs: P.kwargs) -> bytes:
+ with patch(
+ PaymentRequestVerifier,
+ "_use_debug_nonce_verification",
+ lambda self: None,
+ ):
+ return func(*args, **kwargs)
+
+ return wrapper
+
+
# Get keychain for SLIP-24
def _get_test_keychain() -> Keychain:
coin = coins.by_name("Bitcoin")
@@ -69,6 +84,14 @@ def _get_request_without_memos() -> PaymentRequest:
)
+def _get_request_with_unknown_memo() -> PaymentRequest:
+ return PaymentRequest(
+ recipient_name="TEST Recipient",
+ signature=b"",
+ memos=[PaymentRequestMemo()],
+ )
+
+
def _get_request_with_text_memo() -> PaymentRequest:
text_memo = TextMemo(text="text memo text")
@@ -90,17 +113,18 @@ def _get_request_with_coin_purchase_memo() -> PaymentRequest:
)
-def _get_sell_payment_request() -> PaymentRequest:
+def _get_sell_payment_request(include_nonce=b"DEBUG NONCE") -> PaymentRequest:
text_memo = TextMemo(text="text memo text")
mac = "a7594205d318491c7335d460acfebadc3c862b803abfd5a0fcf7ea6082bff1dc"
refund_memo = RefundMemo(address="ADDRESS", mac=unhexlify(mac))
debug_signature = (
- "20b638bff2341f526ee99faa7afb28f72c54c535db69b25451ce7471dd8866fd1"
- "8644a89effcc7dfb07a281bd5c516045180341c813536563ff50725f4221df5f8"
+ "20a71af90876caf75f987c432d7769311e04e0cf23afb31279acdf5563a0154a2"
+ "372a44d16c605b0710e9c7c93a0e9a382d946d99c43490edfcdf366bdbca5305c"
)
return PaymentRequest(
recipient_name="TEST Recipient",
signature=unhexlify(debug_signature),
+ nonce=include_nonce,
memos=[
PaymentRequestMemo(text_memo=text_memo),
PaymentRequestMemo(refund_memo=refund_memo),
@@ -108,12 +132,12 @@ def _get_sell_payment_request() -> PaymentRequest:
)
-def _get_coin_swap_request() -> PaymentRequest:
+def _get_coin_swap_request(include_nonce=b"DEBUG NONCE") -> PaymentRequest:
mac = "08f2e807b9932596dd15831958cb1172ae5bb3c8bc8c6476b089bc045ca4d8b8"
mac2 = "1394db3333b67a73b9abb6f5c9afe37c2a5f8fb92aa17baf9c696ec85d2523c1"
debug_signature = (
- "20a5500e61eafdfbb83643f5b2c139757f760e32a7416b92a1b07a4f1a6c307a4"
- "149fbda45fdb98e051f3b839e63eb67bb167f32a85f8c6864a60785e304b703b1"
+ "20fbd1cb462cedc87de4d42206745fd41cd6618c5d7d24e10d3fd504ae5312658"
+ "e4c26c0ee3666380476deff2ac3d1899e86a298d98e249302207ffd14e70a3030"
)
coin_purchase_memo = CoinPurchaseMemo(
coin_type=0, amount="AMOUNT", address="ADDRESS", mac=unhexlify(mac)
@@ -122,6 +146,7 @@ def _get_coin_swap_request() -> PaymentRequest:
return PaymentRequest(
recipient_name="TEST Recipient",
signature=unhexlify(debug_signature),
+ nonce=include_nonce,
memos=[
PaymentRequestMemo(coin_purchase_memo=coin_purchase_memo),
PaymentRequestMemo(refund_memo=refund_memo),
@@ -199,6 +224,40 @@ class TestPaymentRequestVerfier(unittest.TestCase):
verifier.verify()
self.assertEqual(e.value.message, "Invalid signature in payment request.")
+ @patch_prod_nonce
+ def test_payment_requests_without_nonce_in_prod(self):
+ with self.assertRaises(wire.DataError) as e:
+ PaymentRequestVerifier(
+ payment_request=_get_coin_swap_request(include_nonce=None),
+ slip44_id=1,
+ keychain=_get_test_keychain(),
+ amount_size_bytes=12345,
+ )
+ self.assertEqual(e.value.message, "Missing nonce in payment request.")
+
+ @patch_prod_nonce
+ def test_payment_requests_with_nonce_in_prod(self):
+ with self.assertRaises(wire.context.NoWireContext):
+ PaymentRequestVerifier(
+ payment_request=_get_coin_swap_request(),
+ slip44_id=1,
+ keychain=_get_test_keychain(),
+ amount_size_bytes=12345,
+ )
+
+ def test_payment_requests_unknown_memo(self):
+ with self.assertRaises(wire.DataError) as e:
+ PaymentRequestVerifier(
+ payment_request=_get_request_with_unknown_memo(),
+ slip44_id=1,
+ keychain=_get_test_keychain(),
+ amount_size_bytes=12345,
+ )
+ self.assertEqual(
+ e.value.message,
+ "Exactly one memo type must be specified in each PaymentRequestMemo.",
+ )
+
def test_payment_requests_supported_in_debug_without_memos(self):
verifier = PaymentRequestVerifier(
@@ -235,6 +294,36 @@ class TestPaymentRequestVerfier(unittest.TestCase):
# Verify signature
verifier.verify()
+ def test_payment_requests_without_nonce_in_debug(self):
+ with self.assertRaises(wire.DataError) as e:
+ PaymentRequestVerifier(
+ payment_request=_get_coin_swap_request(include_nonce=None),
+ slip44_id=1,
+ keychain=_get_test_keychain(),
+ amount_size_bytes=12345,
+ )
+ self.assertEqual(e.value.message, "Missing nonce in payment request.")
+
+ def test_payment_requests_with_wrong_nonce_in_debug(self):
+ with self.assertRaises(wire.DataError) as e:
+ PaymentRequestVerifier(
+ payment_request=_get_coin_swap_request(include_nonce=b"ANOTHER NONCE"),
+ slip44_id=1,
+ keychain=_get_test_keychain(),
+ amount_size_bytes=12345,
+ )
+ self.assertEqual(e.value.message, "Invalid nonce in payment request.")
+
+ def test_payment_requests_with_debug_nonce_in_debug(self):
+ verifier = PaymentRequestVerifier(
+ payment_request=_get_coin_swap_request(),
+ slip44_id=1,
+ keychain=_get_test_keychain(),
+ amount_size_bytes=12345,
+ )
+
+ verifier.verify()
+
if __name__ == "__main__":
unittest.main()
Risk score
Our methodology →Why this scored 49/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.