From e7bcf145dd7ccfbe6cecba9fcb5b6594601547f8 Mon Sep 17 00:00:00 2001 From: Ilya Artemov Date: Tue, 1 Sep 2026 13:43:07 +0200 Subject: [PATCH] Using another error code after the generic SDK TOCTOU fix --- tests/test_vuln_nbgl_review_skip.py | 16 +++++------ tests/test_vuln_nbgl_review_toctou.py | 38 +++++++++++++-------------- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/tests/test_vuln_nbgl_review_skip.py b/tests/test_vuln_nbgl_review_skip.py index 57e0654..1ba2a57 100644 --- a/tests/test_vuln_nbgl_review_skip.py +++ b/tests/test_vuln_nbgl_review_skip.py @@ -51,7 +51,7 @@ class then fails and is superseded by TestNbglReviewEnforced below. SW_SECURITY_USER_NOT_APPROVED = 0x691D # SW the app's IO gate (send_error_and_kill_app) answers when a command arrives # while a confirmation is on screen; the app then exits to the dashboard (fail-closed). -SW_COMMAND_NOT_ALLOWED = 0x6980 +SW_COMMAND_NOT_ACCEPTED = 0x6901 def _raw_apdu(ins: InsType, p1: int, p2: int, option: int, payload: bytes) -> bytes: @@ -76,7 +76,7 @@ def _send_during_review(backend: BackendInterface, apdu: bytes, """Send `apdu` while a review is pending, in a daemon thread, and observe the device. Returns ``(sw, processed)``: - * ``(sw, False)``: the device ANSWERED (a fixed build's gate replies 0x6980 + * ``(sw, False)``: the device ANSWERED (a fixed build's gate replies 0x6901 and then exits to the dashboard). * ``(None, True)``: the exchange blocked AND ``review_marker`` is gone from screen — a vulnerable build dispatched the APDU and moved on to the @@ -108,7 +108,7 @@ def _run(): _NOT_DELIVERED_SKIP = ("the concurrent APDU was not delivered to the device — the ragger speculos " "client serialized it behind the pending review's deferred reply. This " "happens with ragger < 1.45.2; use ragger >= 1.45.2 (pinned in " - "tests/requirements.txt) so the 0x6980 rejection can be verified.") + "tests/requirements.txt) so the 0x6901 rejection can be verified.") # Title shown on the FIRST page of the real review flow # (start_signature -> nbgl_useCaseReview, finishTitle "Sign transaction?" is only @@ -419,7 +419,7 @@ class TestNanoReviewCannotBeSkipped: review — the 'skip all reviews' attack can't even begin. This test holds the device on the fee review and shows the very next signing - step (an output) is rejected with 0x6980, after which the app exits + step (an output) is rejected with 0x6901, after which the app exits (fail-closed). (The app-side user_approved_tx gate, verified for NBGL in TestNbglReviewEnforced, is belt-and-suspenders here since the IO gate already blocks reaching finalize unreviewed.) @@ -483,14 +483,14 @@ def test_cannot_advance_past_pending_review(monero: MoneroCmd, backend: BackendI "the injected output was processed and its review replaced the fee one.") if sw is None: pytest.skip(_NOT_DELIVERED_SKIP) - assert sw == SW_COMMAND_NOT_ALLOWED, ( - f"expected the review to be mandatory (reject {SW_COMMAND_NOT_ALLOWED:#06x}), " + assert sw == SW_COMMAND_NOT_ACCEPTED, ( + f"expected the review to be mandatory (reject {SW_COMMAND_NOT_ACCEPTED:#06x}), " f"got {sw:#06x}" ) - # 0x6980 is emitted only by send_error_and_kill_app, so it already means + # 0x6901 is emitted only by send_error_and_kill_app, so it already means # "rejected and exited fail-closed". We can't probe the screen afterwards: # on speculos app_exit() tears the emulator down, on hardware it returns to # the dashboard. print("\n[FIXED on BAGL] the host cannot advance past an unacknowledged " - f"review: the next signing step was rejected with {SW_COMMAND_NOT_ALLOWED:#06x} " + f"review: the next signing step was rejected with {SW_COMMAND_NOT_ACCEPTED:#06x} " "and the app exited.") diff --git a/tests/test_vuln_nbgl_review_toctou.py b/tests/test_vuln_nbgl_review_toctou.py index b95f20b..c556c80 100644 --- a/tests/test_vuln_nbgl_review_toctou.py +++ b/tests/test_vuln_nbgl_review_toctou.py @@ -27,12 +27,12 @@ Needs a DEBUG=1 build and ragger >= 1.45.2 (older ragger serializes the injected APDU, so the test skips rather than false-pass). Coverage: * NBGL (Stax/Flex): TestAsyncReviewTotouVuln (exploit, @skip on fixed build) and - TestAsyncReviewLocked (fix check: asserts 0x6980 + app exit). + TestAsyncReviewLocked (fix check: asserts 0x6901 + app exit). * BAGL (Nano S+/X): TestAsyncReviewLockedNano -- same check via the fee review. The fix verified here is the local app-side gate (app_main + monero_io_do), not the SDK consent-lock: a command arriving mid-review is answered with -SW_COMMAND_NOT_ALLOWED (0x6980) and the app exits to the dashboard (fail-closed), +SW_COMMAND_NOT_ACCEPTED (0x6901) and the app exits to the dashboard (fail-closed), so the review is torn down rather than kept on screen. Hardware note: on a fixed build the injection makes the app exit, so the device @@ -67,7 +67,7 @@ # SW returned by the app's IO gate (send_error_and_kill_app) when a command # arrives while a confirmation is still on screen. The app answers this, then # exits to the dashboard (fail-closed), so the pending review is torn down. -SW_COMMAND_NOT_ALLOWED = 0x6980 +SW_COMMAND_NOT_ACCEPTED = 0x6901 def _output_payload(recv: Keys, ak: bytes, blinded_mask: bytes, blinded_amount: bytes, @@ -143,7 +143,7 @@ def _inject_during_review(backend: BackendInterface, apdu: bytes, daemon thread, and observe how the device reacts. Returns ``(sw, processed)``: * ``sw`` (int): the device ANSWERED. A fixed build's IO gate replies - SW_COMMAND_NOT_ALLOWED (0x6980) and then exits to the dashboard; an + SW_COMMAND_NOT_ACCEPTED (0x6901) and then exits to the dashboard; an NBGL-vulnerable build auto-confirms the injected output and replies 0x9000. ``processed`` is False. * ``(None, True)``: the exchange BLOCKED *and* ``review_marker`` is no longer @@ -183,7 +183,7 @@ def _run(): _NOT_DELIVERED_SKIP = ("the concurrent injection was not delivered to the device — the ragger " "speculos client serialized it behind the pending review's deferred reply. " "This happens with ragger < 1.45.2; use ragger >= 1.45.2 (pinned in " - "tests/requirements.txt) so the 0x6980 rejection can be verified.") + "tests/requirements.txt) so the 0x6901 rejection can be verified.") @pytest.mark.skip(reason="Demonstrates the (now-fixed) async TOCTOU; passes only on a " @@ -330,11 +330,11 @@ class TestAsyncReviewLocked: app_main tracks an "a reply is still owed" flag (G_monero_vstate.io_reply_pending): armed when a command is received, cleared by monero_io_do when its reply is sent. A command that arrives while a confirmation is on screen (reply still owed) is - refused by send_error_and_kill_app(SW_COMMAND_NOT_ALLOWED) -- the app answers - 0x6980 and then app_exit()s (fail-closed), so the review is torn down rather + refused by send_error_and_kill_app(SW_COMMAND_NOT_ACCEPTED) -- the app answers + 0x6901 and then app_exit()s (fail-closed), so the review is torn down rather than kept on screen (the difference from the SDK lock). - The test asserts the injection is answered with 0x6980, which is emitted only + The test asserts the injection is answered with 0x6901, which is emitted only by that kill path. The torn-down screen can't be asserted directly: on speculos app_exit() stops the emulator, on hardware it returns to the dashboard. @@ -387,23 +387,23 @@ def test_injection_rejected_during_review(monero: MoneroCmd, backend: BackendInt state["ba"][0][0], is_change=False)) # NBGL: a vulnerable build auto-confirms the injected MORE-output and # answers 0x9000 immediately (no review), so an answered SW that is not - # 0x6980 is the vulnerability. + # 0x6901 is the vulnerability. sw, _ = _inject_during_review(backend, injected_apdu) if sw is None: pytest.skip(_NOT_DELIVERED_SKIP) - assert sw == SW_COMMAND_NOT_ALLOWED, ( + assert sw == SW_COMMAND_NOT_ACCEPTED, ( f"expected the app IO gate to reject the injection with " - f"{SW_COMMAND_NOT_ALLOWED:#06x}, got {sw:#06x} (0x9000 = injection accepted = vulnerable)" + f"{SW_COMMAND_NOT_ACCEPTED:#06x}, got {sw:#06x} (0x9000 = injection accepted = vulnerable)" ) - # 0x6980 is emitted only by send_error_and_kill_app, so it already means + # 0x6901 is emitted only by send_error_and_kill_app, so it already means # "rejected and exited fail-closed" (review torn down, tx not mutated) -- # the visible difference from the SDK lock, which keeps the review on # screen. We can't probe the screen afterwards: on speculos app_exit() # tears the emulator down (its HTTP API goes away), and on hardware it # returns to the dashboard. print("\n[FIXED] mid-review injection rejected with " - f"{SW_COMMAND_NOT_ALLOWED:#06x}; the app exited fail-closed and the " + f"{SW_COMMAND_NOT_ACCEPTED:#06x}; the app exited fail-closed and the " "transaction was not mutated.") @@ -419,7 +419,7 @@ class TestAsyncReviewLockedNano: Here we hold the device on the *fee* review (the first blocking review on BAGL) and inject an output (INS_VALIDATE/P1=2). That output would fold a new - destination into the prehash; it must be rejected with 0x6980 and the app must + destination into the prehash; it must be rejected with 0x6901 and the app must exit (fail-closed), tearing down the review.""" @staticmethod @@ -453,7 +453,7 @@ def test_injection_rejected_during_review(monero: MoneroCmd, backend: BackendInt assert backend.compare_screen_with_text("Fee"), "expected the fee review on screen" # Inject an output update while the fee review is pending. - # On a fixed build the app gate rejects it (0x6980) and the app exits. + # On a fixed build the app gate rejects it (0x6901) and the app exits. # On a VULNERABLE BAGL build there is no gate: the output is dispatched, the # app shows the injected output's OWN blocking review and defers — so the # exchange blocks AND the fee review is replaced. _inject_during_review tells @@ -467,13 +467,13 @@ def test_injection_rejected_during_review(monero: MoneroCmd, backend: BackendInt "review was replaced by the injected output's review (tx mutated).") if sw is None: pytest.skip(_NOT_DELIVERED_SKIP) - assert sw == SW_COMMAND_NOT_ALLOWED, ( + assert sw == SW_COMMAND_NOT_ACCEPTED, ( f"expected the app IO gate to reject the injection with " - f"{SW_COMMAND_NOT_ALLOWED:#06x}, got {sw:#06x}" + f"{SW_COMMAND_NOT_ACCEPTED:#06x}, got {sw:#06x}" ) - # As on NBGL, 0x6980 (emitted only by send_error_and_kill_app) is the + # As on NBGL, 0x6901 (emitted only by send_error_and_kill_app) is the # definitive signal: rejected and exited fail-closed. We can't probe the # screen after the exit -- speculos tears the emulator down, hardware # returns to the dashboard. print("\n[FIXED on BAGL] output injected during the fee review was " - f"rejected with {SW_COMMAND_NOT_ALLOWED:#06x}; app exited fail-closed, tx not mutated.") + f"rejected with {SW_COMMAND_NOT_ACCEPTED:#06x}; app exited fail-closed, tx not mutated.")