Fix cerberus low#1029
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1029 +/- ##
========================================
Coverage 59.83% 59.83%
========================================
Files 26 26
Lines 2612 2612
Branches 334 334
========================================
Hits 1563 1563
Misses 1045 1045
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
|
elf sizes
Stack consumption summary (clone_app_stack_consumption)
Stack consumption summary
|
ui_display_privacy_shared_secret() told the user they were providing a "public secret key". The corresponding command path is actually an X25519 derivation that returns a 32-byte shared secret computed from the device-held private key and an external public key, not a public value. On a hardware wallet, approval text is the last security boundary; inaccurate wording can materially change whether a user consents to secret disclosure. Rename the confirmation header to "Provide derived shared secret" so the title accurately describes the secret material being released.
get_eth2_public_key() derived an EIP-2333 BLS12-381 private scalar into the stack buffer privateKeyData but never erased it before returning. The neighboring locals (tmp, privateKey) were explicitly scrubbed in the end: cleanup block, which indicates the omission was accidental. On embedded targets, residual stack contents can persist across calls and become recoverable through later memory-disclosure bugs, crash dumps, or forensic extraction. Exposure of validator private keys can lead to unauthorized signing or loss of staking/account control. Add explicit_bzero(privateKeyData, sizeof(privateKeyData)) to the existing cleanup block.
process_chain_id() accepted chainId fields up to INT256_LENGTH (32 bytes) for EIP-2930 / EIP-1559 / EIP-7702 transactions, but downstream get_tx_chain_id() converts the stored value to uint64_t via u64_from_BE() which silently consumes only the first 8 bytes. The signature hash still covers the full original encoding, so a malicious host could craft a transaction whose signature applies to one chain while the device displays / validates only the truncated 64-bit prefix. Restrict the field length validation to sizeof(uint64_t). Any chain ID that does not fit in a uint64_t is now rejected at the parser level rather than being truncated, keeping the UI / network selection / chain consistency checks aligned with what gets signed.
The app exposes a "Always display the transaction or message hash" setting and the EIP-191 signing path computes the message hash before launching the review. However, the NBGL personal-message review never consulted N_storage.displayHash and never added the hash to the review pairs. Users who explicitly enabled hash display did not actually see the hash for personal-message signing, reducing their ability to verify the exact bytes being signed when rendering is ambiguous (whitespace, encoding differences, long content, hex fallback). Mirror the transaction-hash flow already used in ui_approve_tx: when N_storage.displayHash is set, format tmpCtx.messageSigningContext.hash into strings.common.tx_hash and append it as a second review pair labelled "Message hash". The setting toggle now applies consistently to EIP-191 messages, EIP-712 typed data, and transactions.
handle_perform_privacy_operation() stores the derived X25519 shared
secret in tmpCtx.publicKeyContext.publicKey and, on the confirmation
path, copies a hex view of the secret into strings.common.fullAmount
for UI rendering. Neither global was scrubbed before returning:
set_result_perform_privacy_operation() only copied the secret into the
APDU reply buffer, successful synchronous APDUs were not followed by
reset_app_context(), and reset_app_context() itself did not clear the
strings buffer.
As a result, the last derived shared secret could remain resident in
RAM across later APDU exchanges, where a memory-disclosure bug, crash
dump, fault injection, or forensic extraction could recover it.
Two changes, kept together so the residue is closed end-to-end:
* logic_perform_privacy_operation.c: scrub tmpCtx.publicKeyContext
immediately after the secret has been copied to G_io_tx_buffer.
* main.c: zero the whole strings struct in reset_app_context() so
any UI-formatted view of secrets (and other transient strings)
does not persist across resets.
decode_function_selector() automatically queried 4byte.directory for any selector that was not present in the local cache. Operators running the decoder on captured APDU traces had no way to keep the analysis offline, and the function selectors of internal contracts or private integrations could leak to a third-party host without an explicit prompt. Add an --online-selectors flag (default off). When set, main() flips the module-level ALLOW_ONLINE_SELECTOR_LOOKUP gate and emits a clear warning before any external request. Without the flag the decoder returns "Unknown (0x...)" for cache misses and never reaches the network (CWE-201).
InputData.process_data() reset current_path on entry but left filtering_paths, filtering_tokens, filtering_calldatas and sig_ctx populated whenever the `filters` argument was omitted. As a result, a prior filtered signing flow contaminated later supposedly-unfiltered flows in the same Python process: the helper would replay leftover filter descriptors as extra APDUs and corrupt downstream snapshot comparisons or assertions. Clear every piece of module-level state at the start of each call, regardless of whether filters are provided. This makes the helper re-entrant within a single test process so cross-test state leakage no longer hides UI/signing regressions (CWE-664).
cedelavergne-ledger
force-pushed
the
cev/fix_cerberus_low
branch
from
946cf7a to
89db014
Compare
apaillier-ledger
approved these changes
May 12, 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
3 participants
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.
Description
https://ledgerhq.atlassian.net/browse/B2CA-2627
Changes include