Repository navigation
chore: sync with LedgerHQ/app-cosmos v2.39.3 - #214
Merged
Merged
Conversation
Merge develop into main following B2CA-2666
Merge develop into master (B2CA-2666)
Align master with P1
Bump version 2.38.19
Review fidelity, parser hardening, and Babylon wrapped staking support
…lares extractHDPath hard-coded 118' and 60', so a chain registering its own SLIP-0044 coin type could not be derived at all. Derive the accepted set from chainConfig[] instead: such a chain now needs nothing beyond its table entry, matching the table's own comment. Add Gonka (1200', hrp gonka) as one entry. The existing HRP-first lookup in checkChainConfig already refuses a declared HRP on any path other than its own, so gonka is refused on 118' the same way inj already is -- no separate exclusivity check is needed for it. Require the HRP on every coin type other than 118'. Only the generic Cosmos path has a meaningful default; elsewhere an omitted HRP would derive a key on that chain's path while defaulting the review screen to a cosmos1... address. Declare 44'/1200' in PATH_APP_LOAD_PARAMS -- the OS enforces this independently of the checks above; without it the device answers a generic 0x6400 that no code review can predict. Swap check_address now answers only for chains the app can also price and verify at signing time, instead of relying on a later chain-id mismatch. LIVE-36205
The version bump left the golden snapshots on 2.39.1, so the main menu test fails on the version screen on all five devices. Regenerates the *-mainmenu snapshots, the only ones the version screen appears in: 00004.png everywhere, plus 00010.png on nanox and nanos+ where the menu wraps around to it a second time -- compareSnapshots stops at the first mismatch, so that second one is never reported. Nothing else moved: the full suite passes at 200/200. LIVE-36205
feat(chain-config): add Gonka coin type 1200 with exclusive HRP resolution
Release 2.39.1 (stax_1.10.1_2.39.1_sdk_v26.6.1): merge into main
fix: audit findings — display fidelity and textual encoding
…rror names (#6) * fix: name every parser error code parser_getErrorDescription had no case for six members of parser_error_t, so each of them reached the host as "Unrecognized error code" -- including parser_unexpected_number_items, which is what a transaction carrying more items than the review can reach is refused with. Whoever was holding the device was told nothing about why it would not sign. Add the six missing cases and pin the rule with a test that walks the enum: the default belongs to values outside it, not to members of it. * fix(display): keep a large executed batch signable Giving the messages a MsgExec carries their own screens costs three or four review items each, and the review index stops at MAX_REVIEW_ITEMS. A MsgExec with 32 executed messages reviewed and signed before those screens existed -- as four items, the executed messages being one paged JSON value -- and was refused outright afterwards. 31 comes to exactly 127; expert mode adds four items of its own and moves the boundary down by one. Authz grantee batches are the realistic case. Where the detail does not fit, fall back to the rendering it replaced rather than refuse the document. Nothing is hidden by doing so: the executed messages are still shown in full, one page at a time, which is how they were shown before. The choice is a property of the document, taken once while indexing and read back by both walks, so the item count and the rendering always enumerate the same set. A document too large to review even as a blob is still refused -- falling back is not truncating, and an item the review cannot reach must never be counted. The index is now also keyed on expert mode, which decides how many items the count includes and so which side of the boundary a document falls on. * fix(display): keep an escape that decodes outside printable ASCII Decoding escapes while paging refused anything below 0x20 and emitted everything above it as UTF-8. So the line separators, the C1 controls, the zero-width characters and the bidi overrides reached the screen as the characters they name: nothing at all, or a change of reading direction. A memo carrying U+2028 between two runs of digits used to render the escape and now reads as one number -- and U+2028 and U+2029 are exactly the code points Go's encoder escapes, so that is the spelling an ordinary document arrives in. Decode only while it produces a character the screen can show. Above printable ASCII, copy the escape through as the document spells it, which is what the review drew before it decoded anything, and which names the code point instead of hiding it. The escape for '>' still becomes '>', so the Go-escaped HTML characters that made decoding necessary are unaffected, and an escape below 0x20 is still refused. Copying through never grows a value: a unicode escape is six bytes in and six out, a surrogate pair twelve and twelve, so a page still fits the buffer it always did. huge_test's expectations are regenerated from actual output; its memo carries both spellings and is shown the way it was before the decoding, across the two pages it took then. Raw non-ASCII bytes remain accepted as they always were -- a separate, older gap recorded in tests/control_bytes.cpp, not touched here. * fix: hold the device while a screen owns it G_io_apdu_buffer is where the next incoming APDU lands, and app_main hands one over as soon as it arrives -- which, for a handler that returned IO_ASYNCH_REPLY, is while the user is still reading the review. The address was derived into that buffer, paged out of it at draw time and replied out of it at approve time, so a request arriving mid-review replaced the address on screen and the bytes the approval returned. tx_json.own_addr, held for the whole signing review, pointed into it too. Keep the answer in app RAM instead, and copy it into the APDU buffer only when the reply is actually sent. Then gate handleApdu on zxlib v52's view_review_is_pending(), which the app took the release for but never adopted. It covers the error modals as well: a handler that raises one has already thrown, so the host has its status word, but the modal still owes a reply through h_error_accept -- and left unlocked, a review drawn on top of it collects the press the user meant as "dismiss". COMMAND_NOT_ALLOWED is the refusal on purpose: handleApdu already leaves the in-flight request's state alone for exactly that status word. Not exercised on the simulator: speculos serves one APDU connection at a time, so a request made while another is unanswered is refused by the transport and never reaches the app. The paths this changes are covered by the existing address and signing tests; the race itself needs a real device.
Merge develop into main
Audit findings — display fidelity, textual encoding and review lock
Merge main into develop
Merge develop into main
|
This PR does not match any of the 1 configured review trigger rule. |
emmanuelm41
approved these changes
Oct 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Brings
mainin sync with the publicLedgerHQ/app-cosmosmain, which carries v2.39.3, the version Ledger released.It contains:
ledger-secure-sdksubmodule dropped, and new unit and Zemu testsHow it was merged
Our
mainhas one commit Ledger's does not (chore: inherit org security policy (#211)), so this is a merge ofLedgerHQ/app-cosmos@mainintomain, with no conflicts. The resulting tree is identical to Ledger's except thatSECURITY.mdstays removed, as #211 intended.Merge note
Please merge with a merge commit (not squash or rebase), so future syncs keep Ledger's history.