Skip to content

Merge develop into master - #429

Merged
yogh333 merged 5 commits into
masterfrom
develop
Aug 26, 2026
Merged

Merge develop into master#429
yogh333 merged 5 commits into
masterfrom
develop

Conversation

@yogh333

@yogh333 yogh333 commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

No description provided.

sportron-ledger and others added 5 commits August 21, 2026 10:37
`next_event_ahead` is called from `ux_sync_wait` in two different
situations, and rejecting unconditionally broke the first one:

* no command being processed (idle home screen): an incoming APDU is the
  normal way of receiving work. Returning `false` here meant
  `ux_sync_wait(true)` could never report `UxSyncRetApduReceived`, so an
  app using the blocking `NbglHomeAndSettings::show()` answered 0x6901 to
  every APDU and became unreachable from the host.
* a command being processed (review, status, ... screen): an incoming
  APDU is a double APDU and must be answered CmdNotAccepted.

io_legacy had no state distinguishing the two: `event_pending` cannot
serve, as `check_event` clears it before handing the command to the
application. Add `Comm::apdu_in_progress`, set when a command is handed
to the application and cleared when the application replies, and gate
the rejection on it. This also lets `apdu_send` stop clearing
`event_pending`, which silently discarded queued commands on every reply
path instead of answering them.

Take the decision on the raw frame rather than after `detect_apdu`:
`decode_event` overwrites `apdu_buffer`, `apdu_type`, `rx` and
`rx_length`, i.e. state owned by the command still being processed.
BOLOS APDUs (CLA 0xB0) and frames too short to hold a header keep
falling through to `check_event` so their existing handling is
preserved.

Reject via a dedicated `reject_apdu` that transmits the status word on
the intruder's own transport and leaves every response and receive field
untouched. Going through `Comm::reply`/`apdu_send` instead would flush
any bytes the application had staged before showing the screen (sending
them to the host with the error, then losing them from the real
response), overwrite `apdu_type` so the in-flight command replied on the
wrong channel, and consume a SEPH event, which can drop the user's tap.

Also mark `NbglHomeAndSettings::show()` with `#[deprecated]`, which its
documentation already stated in prose only, and bump to 1.37.0: the new
`StatusWords::CmdNotAccepted` variant breaks downstream exhaustive
matches over that public enum.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Mirror the io_legacy change in the io_new backend, and fix three
behaviours of the double-APDU rejection there.

`Comm::pending_apdu` was used as the trigger, but it means "an APDU was
decoded ahead and not consumed yet", not "a command is being processed".
Rejecting on it deferred the answer to the *next* polling iteration, so
a screen completing in between discarded the intruder with no response
at all: `CommandResponse::send` clears `pending_apdu` unconditionally.
Add `Comm::apdu_in_progress`, set when a command is handed to the
application and cleared when the application replies, and reject on the
iteration that detects the APDU. The `pending_apdu` branch is kept for
the case where no command is in flight and the displayed screen does not
exit on APDU: answering there is what lets the polling loop, and so the
screen, keep running.

Stop gating the inline BOLOS (CLA 0xB0) handling behind `stack_usage`.
Default builds set `pending_apdu` for those APDUs and then answered
CmdNotAccepted, whereas `next_command` dispatches them unconditionally,
so OS level requests failed while a screen was displayed. Save and
restore `apdu_in_progress` around the inline call, as the BOLOS reply
goes through `begin_response().send()` and would otherwise be taken for
the reply to the command still being processed.

Answer `DecodedEventType::ApduError` instead of ignoring it: a malformed
APDU arriving during a screen used to fall into the catch-all arm and
get no status word, leaving the host waiting, while `next_command` and
io_legacy both reply BadLen.

Reject through a new `Comm::reject_apdu`, which transmits the status
word on the transport the rejected APDU arrived on and stages nothing
into the shared buffer. `decode_apdu` overwrites `apdu_type` before the
rejection is decided, so it is captured on entry and restored on every
path that does not deliver the APDU to the application; otherwise the
in-flight command's response was sent on the intruder's channel. This
also removes the `Reply(StatusWords::CmdNotAccepted as u16)` cast and
the call back through `reply_status_impl`, which re-resolved the erased
global to build a second `&mut Comm` aliasing the one already in hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fix: If incoming APDU occurs during previous APDU processing then send a specific error code.
@yogh333
yogh333 requested a review from a team as a code owner August 26, 2026 13:08
@yogh333
yogh333 requested review from mbrousset-ledger and removed request for a team August 26, 2026 13:08
@yogh333
yogh333 merged commit 8057ecc into master Aug 26, 2026
204 of 205 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants