Merge pull request #1029 from LedgerHQ/cev/fix_cerberus_low

Fix cerberus low

Author: cedelavergne-ledger

client/src/ledger_app_clients/ethereum/eip712/InputData.py

@@ -595,8 +595,23 @@ def process_data(aclient: EthAppClient,
                  filters: Optional[dict] = None) -> None:
     global app_client
     global current_path
-
+    global filtering_paths
+    global filtering_tokens
+    global filtering_calldatas
+    global sig_ctx
+
+    # Reset every piece of module-level state at the start of each call so
+    # that a previous filtered run cannot contaminate the next one. The
+    # previous behavior reset current_path but left filtering_paths,
+    # filtering_tokens, filtering_calldatas and sig_ctx populated whenever
+    # `filters` was omitted, leading to silent cross-test state leakage
+    # (CWE-664).
     current_path = []
+    filtering_paths = {}
+    filtering_tokens = []
+    filtering_calldatas = []
+    sig_ctx = {}
+
     # deepcopy because this function modifies the dict
     data_json = copy.deepcopy(data_json)
     app_client = aclient

src/features/get_eth2_public_key/cmd_get_eth2_public_key.c

@@ -54,6 +54,11 @@ uint32_t get_eth2_public_key(uint32_t *bip32Path, uint8_t bip32PathLength, uint8
     publicKey.W[1] |= 0x80 | yFlag;
     memmove(out, publicKey.W + 1, BLS12381_G1_COMPRESSED_PUBKEY_LENGTH);
 end:
+    // privateKeyData holds the EIP-2333-derived BLS12-381 private scalar (32
+    // bytes of secret material). Residual stack content can be recovered
+    // through later memory-disclosure bugs, crash dumps, or forensic
+    // extraction, so wipe it before returning (CWE-226).
+    explicit_bzero(privateKeyData, sizeof(privateKeyData));
     explicit_bzero(tmp, BLS12381_G1_UNCOMPRESSED_PUBKEY_LENGTH);
     explicit_bzero((void *) &privateKey, sizeof(cx_ecfp_256_extended_private_key_t));
     return error;

src/features/perform_privacy_operation/logic_perform_privacy_operation.c

@@ -4,5 +4,9 @@ uint32_t set_result_perform_privacy_operation() {
     for (uint8_t i = 0; i < INT256_LENGTH; i++) {
         G_io_tx_buffer[i] = tmpCtx.publicKeyContext.publicKey.W[INT256_LENGTH - i];
     }
+    // Scrub the source as soon as the secret has been copied to the reply
+    // buffer. For the P2_SHARED_SECRET path this is X25519 secret material;
+    // we don't want it lingering in tmpCtx until the next reset (CWE-312).
+    explicit_bzero(&tmpCtx.publicKeyContext, sizeof(tmpCtx.publicKeyContext));
     return INT256_LENGTH;
 }

src/features/perform_privacy_operation/ui_common_perform_privacy_operation.c

@@ -1,12 +1,25 @@
 #include "apdu_constants.h"
+#include "shared_context.h"
 #include "ui_callbacks.h"
 #include "feature_perform_privacy_operation.h"
 
+// The shared-secret review path renders the X25519 derived secret as hex into
+// strings.common.fullAmount, and the corresponding device address into
+// strings.common.toAddress. Scrub them on every exit so the secret view does
+// not linger in RAM (CWE-312). Surgical instead of a global strings reset:
+// other flows reuse the same union for non-sensitive review state.
+static void scrub_privacy_strings(void) {
+    explicit_bzero(strings.common.fullAmount, sizeof(strings.common.fullAmount));
+    explicit_bzero(strings.common.toAddress, sizeof(strings.common.toAddress));
+}
+
 unsigned int io_seproxyhal_touch_privacy_ok(void) {
     uint32_t tx = set_result_perform_privacy_operation();
+    scrub_privacy_strings();
     return io_seproxyhal_send_status(SWO_SUCCESS, tx, true, true);
 }
 
 unsigned int io_seproxyhal_touch_privacy_cancel(void) {
+    scrub_privacy_strings();
     return io_seproxyhal_send_status(SWO_CONDITIONS_NOT_SATISFIED, 0, true, true);
 }

src/features/sign_tx/eth_ustream.c

@@ -168,7 +168,18 @@ static bool process_auth_list(txContext_t *context) {
 }
 
 static bool process_chain_id(txContext_t *context) {
-    if (check_fields(context, "RLP_CHAINID", INT256_LENGTH) == false) {
+    // Reject chain IDs that would not fit in a uint64_t. Downstream
+    // get_tx_chain_id() converts the stored bytes through u64_from_BE(),
+    // silently truncating anything beyond 8 bytes — so a 32-byte field
+    // could let the signature cover one chain while the UI / consistency
+    // checks operate on the truncated 64-bit prefix (CWE-197).
+    //
+    // This is a hardening, not a regression: EIP-2294 recommends bounding
+    // chain_id to 2^53 for interoperability, and no production EVM chain
+    // today uses a value beyond uint64_t. Pre-fix, anything beyond 8 bytes
+    // was still hashed into the signature but never matched on display, so
+    // there is no legitimate flow this rejection breaks.
+    if (check_fields(context, "RLP_CHAINID", sizeof(uint64_t)) == false) {
         return false;
     }
 

src/nbgl/ui_display_privacy.c

@@ -39,5 +39,9 @@ void ui_display_privacy_public_key(void) {
 }
 
 void ui_display_privacy_shared_secret(void) {
-    buildFirstPage("Provide public\nsecret key");
+    // The value released here is the X25519 shared secret derived from the
+    // device-held private key and the host-supplied peer public key — it is
+    // NOT a public value. Wording must make that clear so the user does not
+    // approve secret disclosure thinking it is a public-key export.
+    buildFirstPage("Provide derived\nshared secret");
 }

src/nbgl/ui_sign_message.c

@@ -21,8 +21,27 @@ static void ui_191_finish_cb(bool confirm) {
 }
 
 void ui_191_start(const char *message) {
-    // Initialize the buffers
-    if (!ui_pairs_init(1)) {
+    // Honor the "Always display the transaction or message hash" setting for
+    // EIP-191 personal messages so that the user can verify the exact bytes
+    // being signed even when rendering is ambiguous (whitespace, encoding,
+    // long content, hex fallback). The tx hash flow does this already; the
+    // message review used to silently ignore the toggle (CWE-451).
+    //
+    // Format the hash first and only count it as a pair if formatting
+    // succeeded — otherwise we would advertise a 2-pair list with an
+    // uninitialized g_pairs[1] and the device would render garbage.
+    bool show_hash = false;
+    if (N_storage.displayHash) {
+        strlcpy(strings.common.tx_hash, "0x", 3);
+        if (bytes_to_lowercase_hex(strings.common.tx_hash + 2,
+                                   sizeof(strings.common.tx_hash) - 2,
+                                   tmpCtx.messageSigningContext.hash,
+                                   INT256_LENGTH) >= 0) {
+            show_hash = true;
+        }
+    }
+
+    if (!ui_pairs_init(show_hash ? 2 : 1)) {
         // Initialization failed, cleanup and return
         return;
     }
@@ -44,8 +63,13 @@ void ui_191_start(const char *message) {
              ui_tx_simulation_finish_str());
 
     g_pairsList->wrapping = true;
-    g_pairs->item = "Message";
-    g_pairs->value = message;
+    g_pairs[0].item = "Message";
+    g_pairs[0].value = message;
+
+    if (show_hash) {
+        g_pairs[1].item = "Message hash";
+        g_pairs[1].value = strings.common.tx_hash;
+    }
 
 #ifndef FUZZ
     nbgl_useCaseAdvancedReview(TYPE_MESSAGE,

tools/decode_apdu.py

@@ -26,6 +26,11 @@
 
 # Local selector cache at module level
 LOCAL_SELECTORS = {}
+# Whether to fall back to 4byte.directory for selectors absent from the local
+# cache. Opt-in only — set from --online-selectors in main() so that running
+# the decoder over a captured APDU trace does not silently disclose its
+# function selectors to a third-party host (CWE-201).
+ALLOW_ONLINE_SELECTOR_LOOKUP = False
 CACHE_FILE = Path(__file__).parent / "function_selectors.json"
 
 logger = logging.getLogger(__name__)
@@ -49,6 +54,15 @@ def init_parser() -> argparse.ArgumentParser:
     parser = argparse.ArgumentParser(description="Decode APDU replay file to extract transaction details.")
     parser.add_argument("--input", "-i", required=True, help="Input apdu replay file.")
     parser.add_argument("--verbose", "-v", action='store_true', help="Verbose mode")
+    parser.add_argument(
+        "--online-selectors",
+        action="store_true",
+        help=(
+            "Allow unknown function selectors to be looked up online against "
+            "4byte.directory. By default the decoder works offline and leaks "
+            "no replay data over the network (CWE-201)."
+        ),
+    )
     return parser
 
 
@@ -123,7 +137,12 @@ def decode_function_selector(selector: str) -> str:
         logger.debug(f"Found selector {selector} in local cache")
         return LOCAL_SELECTORS[selector]
 
-    # 2. Try online API as fallback
+    # 2. Try online API as fallback — only when the user explicitly asked for
+    # it. Without the opt-in we never leak the selector to 4byte.directory.
+    if not ALLOW_ONLINE_SELECTOR_LOOKUP:
+        logger.debug(f"Selector {selector} not in cache; online lookup disabled")
+        return f"Unknown (0x{selector})"
+
     logger.debug(f"Selector {selector} not in cache, querying API...")
     try:
         url = f"https://www.4byte.directory/api/v1/signatures/?hex_signature=0x{selector}"
@@ -1024,11 +1043,21 @@ def parse_apdu_line(line: str) -> Optional[bytes]:
 #          Main entry
 # ===============================================================================
 def main() -> None:
+    global ALLOW_ONLINE_SELECTOR_LOOKUP
+
     parser = init_parser()
     args = parser.parse_args()
 
     set_logging(args.verbose)
 
+    if args.online_selectors:
+        ALLOW_ONLINE_SELECTOR_LOOKUP = True
+        logger.warning(
+            "Online selector lookup enabled: unknown function selectors from "
+            "the replay will be sent to https://www.4byte.directory/. Do not "
+            "use this flag on sensitive traces."
+        )
+
     # Load selector cache at startup
     load_selector_cache()
     logger.debug(f"Loaded {len(LOCAL_SELECTORS)} function selectors from cache")