upstream #384: fault injection hardening, nanopb memleak, BIP39 validation

.github/workflows/ci.yml

@@ -52,22 +52,26 @@ jobs:
         with:
           ref: ${{ github.event.pull_request.head.sha || github.sha }}
 
-      - name: Install clang-format
-        run: sudo apt-get install -y clang-format-14
-
       - name: Check code formatting
         run: |
+          # Use pre-installed clang-format (ubuntu-latest ships with it)
+          CF=$(command -v clang-format-14 || command -v clang-format || true)
+          if [ -z "$CF" ]; then
+            echo "::warning::clang-format not found, skipping lint"
+            exit 0
+          fi
+          echo "Using: $CF ($($CF --version | head -1))"
           FAILED=0
           for f in $(find include/keepkey lib/firmware lib/board lib/transport/src \
                      -name '*.c' -o -name '*.h' 2>/dev/null | grep -v generated | grep -v '.pb.'); do
-            if ! clang-format-14 --style=file --dry-run --Werror "$f" 2>/dev/null; then
+            if ! $CF --style=file --dry-run --Werror "$f" 2>/dev/null; then
               echo "::warning file=$f::Formatting differs from .clang-format"
               FAILED=1
             fi
           done
           if [ "$FAILED" = "1" ]; then
             echo ""
-            echo "Run: clang-format -i <file> to fix formatting"
+            echo "Run: $CF -i <file> to fix formatting"
             echo "Note: treating as warning until codebase is reformatted"
           fi
         # Warn-only until existing code is reformatted

include/keepkey/transport/messages-solana.options

@@ -0,0 +1,17 @@
+SolanaGetAddress.address_n max_count:8
+SolanaGetAddress.coin_name max_size:21
+
+SolanaAddress.address max_size:64
+
+SolanaSignTx.address_n max_count:8
+SolanaSignTx.coin_name max_size:21
+SolanaSignTx.raw_tx max_size:2048
+
+SolanaSignedTx.signature max_size:64
+
+SolanaSignMessage.address_n max_count:8
+SolanaSignMessage.coin_name max_size:21
+SolanaSignMessage.message max_size:1024
+
+SolanaMessageSignature.public_key max_size:32
+SolanaMessageSignature.signature max_size:64

include/keepkey/transport/messages-ton.options

@@ -0,0 +1,12 @@
+TonGetAddress.address_n max_count:8
+TonGetAddress.coin_name max_size:21
+
+TonAddress.address max_size:50
+TonAddress.raw_address max_size:70
+
+TonSignTx.address_n max_count:8
+TonSignTx.coin_name max_size:21
+TonSignTx.raw_tx max_size:1024
+TonSignTx.to_address max_size:50
+
+TonSignedTx.signature max_size:64

include/keepkey/transport/messages-tron.options

@@ -0,0 +1,14 @@
+TronGetAddress.address_n max_count:8
+TronGetAddress.coin_name max_size:21
+
+TronAddress.address max_size:35
+
+TronSignTx.address_n max_count:8
+TronSignTx.coin_name max_size:21
+TronSignTx.raw_data max_size:2048
+TronSignTx.ref_block_bytes max_size:4
+TronSignTx.ref_block_hash max_size:32
+TronSignTx.contract_type max_size:64
+TronSignTx.to_address max_size:35
+
+TronSignedTx.signature max_size:65

include/keepkey/transport/messages.options

@@ -131,3 +131,5 @@ FlashHash.challenge max_size:32
 FlashWrite.data max_size:1024
 
 FlashHashResponse.data max_size:32
+
+Bip85Mnemonic.mnemonic max_size:241

lib/board/signatures.c

@@ -20,7 +20,9 @@
 #include "trezor/crypto/sha2.h"
 #include "trezor/crypto/ecdsa.h"
 #include "trezor/crypto/secp256k1.h"
+#include "trezor/crypto/memzero.h"
 #include "keepkey/board/memory.h"
+#include "keepkey/board/memcmp_s.h"
 #include "keepkey/board/signatures.h"
 #include "keepkey/board/pubkeys.h"
 
@@ -68,23 +70,51 @@ int signatures_ok(void) {
     return KEY_EXPIRED;
   } /* Expired signing key */
 
+  /* F3 hardening: double-compute SHA-256, compare in constant time */
+  uint8_t firmware_fingerprint2[32];
   sha256_Raw((uint8_t *)FLASH_APP_START, codelen, firmware_fingerprint);
+  asm volatile("" ::: "memory");
+  sha256_Raw((uint8_t *)FLASH_APP_START, codelen, firmware_fingerprint2);
 
-  if (ecdsa_verify_digest(&secp256k1, pubkey[sigindex1 - 1],
-                          (uint8_t *)FLASH_META_SIG1,
-                          firmware_fingerprint) != 0) { /* Failure */
+  if (memcmp_s(firmware_fingerprint, firmware_fingerprint2, 32) != 0) {
+    memzero(firmware_fingerprint, sizeof(firmware_fingerprint));
+    memzero(firmware_fingerprint2, sizeof(firmware_fingerprint2));
     return SIG_FAIL;
   }
+  memzero(firmware_fingerprint2, sizeof(firmware_fingerprint2));
 
-  if (ecdsa_verify_digest(&secp256k1, pubkey[sigindex2 - 1],
-                          (uint8_t *)FLASH_META_SIG2,
-                          firmware_fingerprint) != 0) { /* Failure */
+  /* F3 hardening: infective aggregation — accumulate all three ECDSA
+   * results instead of early-returning on each. Forces attacker to
+   * corrupt all three verify calls, not just skip one branch. */
+  volatile int verify_acc = 0;
+  volatile int verify_sentinel = 0;
+
+  verify_acc |= ecdsa_verify_digest(&secp256k1, pubkey[sigindex1 - 1],
+                                    (uint8_t *)FLASH_META_SIG1,
+                                    firmware_fingerprint);
+  verify_sentinel++;
+  asm volatile("" ::: "memory");
+
+  verify_acc |= ecdsa_verify_digest(&secp256k1, pubkey[sigindex2 - 1],
+                                    (uint8_t *)FLASH_META_SIG2,
+                                    firmware_fingerprint);
+  verify_sentinel++;
+  asm volatile("" ::: "memory");
+
+  verify_acc |= ecdsa_verify_digest(&secp256k1, pubkey[sigindex3 - 1],
+                                    (uint8_t *)FLASH_META_SIG3,
+                                    firmware_fingerprint);
+  verify_sentinel++;
+  asm volatile("" ::: "memory");
+
+  memzero(firmware_fingerprint, sizeof(firmware_fingerprint));
+
+  /* All three verifies must have executed and all must have passed */
+  if (verify_sentinel != 3) {
     return SIG_FAIL;
   }
 
-  if (ecdsa_verify_digest(&secp256k1, pubkey[sigindex3 - 1],
-                          (uint8_t *)FLASH_META_SIG3,
-                          firmware_fingerprint) != 0) { /* Failure */
+  if (verify_acc != 0) {
     return SIG_FAIL;
   }
 

lib/firmware/recovery_cipher.c

@@ -462,6 +462,24 @@ void recovery_character(const char *character) {
       }
     }
   } else {
+    /* Per-word BIP39 validation: reject immediately if the decoded word
+     * doesn't match any entry in the wordlist. */
+    if (enforce_wordlist && strlen(decoded_word) > 0) {
+      static CONFIDENTIAL char check_word[CURRENT_WORD_BUF];
+      strlcpy(check_word, decoded_word, sizeof(check_word));
+      bool valid = attempt_auto_complete(check_word);
+      memzero(check_word, sizeof(check_word));
+      if (!valid) {
+        memzero(coded_word, sizeof(coded_word));
+        memzero(decoded_word, sizeof(decoded_word));
+        recovery_cipher_abort();
+        fsm_sendFailure(FailureType_Failure_SyntaxError,
+                        "Word not found in BIP39 wordlist");
+        layoutHome();
+        return;
+      }
+    }
+
     memzero(coded_word, sizeof(coded_word));
     memzero(decoded_word, sizeof(decoded_word));
 
@@ -575,7 +593,7 @@ void recovery_cipher_finalize(void) {
   }
   memzero(temp_word, sizeof(temp_word));
 
-  if (!auto_completed && !enforce_wordlist) {
+  if (!auto_completed && enforce_wordlist) {
     if (!dry_run) {
       storage_reset();
     }

lib/firmware/signing.c

@@ -624,7 +624,7 @@ void signing_init(const SignTx *msg, const CoinType *_coin,
         branch_id = 0x5BA81B19;  // Overwinter
         break;
       case 4:
-        branch_id = 0x76B809BB;  // Sapling
+        branch_id = 0xC8E71055;  // NU6
         break;
     }
   }

lib/transport/CMakeLists.txt

@@ -15,6 +15,9 @@ set(protoc_pb_sources
     ${DEVICE_PROTOCOL}/messages-tendermint.proto
     ${DEVICE_PROTOCOL}/messages-thorchain.proto
     ${DEVICE_PROTOCOL}/messages-mayachain.proto
+    ${DEVICE_PROTOCOL}/messages-solana.proto
+    ${DEVICE_PROTOCOL}/messages-tron.proto
+    ${DEVICE_PROTOCOL}/messages-ton.proto
     ${DEVICE_PROTOCOL}/messages.proto)
 
 set(protoc_pb_options
@@ -29,6 +32,9 @@ set(protoc_pb_options
     ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-tendermint.options
     ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-thorchain.options
     ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-mayachain.options
+    ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-solana.options
+    ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-tron.options
+    ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages-ton.options
     ${CMAKE_SOURCE_DIR}/include/keepkey/transport/messages.options)
 
 set(protoc_c_sources
@@ -43,6 +49,9 @@ set(protoc_c_sources
     ${CMAKE_BINARY_DIR}/lib/transport/messages-tendermint.pb.c
     ${CMAKE_BINARY_DIR}/lib/transport/messages-thorchain.pb.c
     ${CMAKE_BINARY_DIR}/lib/transport/messages-mayachain.pb.c
+    ${CMAKE_BINARY_DIR}/lib/transport/messages-solana.pb.c
+    ${CMAKE_BINARY_DIR}/lib/transport/messages-tron.pb.c
+    ${CMAKE_BINARY_DIR}/lib/transport/messages-ton.pb.c
     ${CMAKE_BINARY_DIR}/lib/transport/messages.pb.c)
 
 set(protoc_c_headers
@@ -57,6 +66,9 @@ set(protoc_c_headers
     ${CMAKE_BINARY_DIR}/include/messages-tendermint.pb.h
     ${CMAKE_BINARY_DIR}/include/messages-thorchain.pb.h
     ${CMAKE_BINARY_DIR}/include/messages-mayachain.pb.h
+    ${CMAKE_BINARY_DIR}/include/messages-solana.pb.h
+    ${CMAKE_BINARY_DIR}/include/messages-tron.pb.h
+    ${CMAKE_BINARY_DIR}/include/messages-ton.pb.h
     ${CMAKE_BINARY_DIR}/include/messages.pb.h)
 
 set(protoc_pb_sources_moved
@@ -71,6 +83,9 @@ set(protoc_pb_sources_moved
     ${CMAKE_BINARY_DIR}/lib/transport/messages-tendermint.proto
     ${CMAKE_BINARY_DIR}/lib/transport/messages-thorchain.proto
     ${CMAKE_BINARY_DIR}/lib/transport/messages-mayachain.proto
+    ${CMAKE_BINARY_DIR}/lib/transport/messages-solana.proto
+    ${CMAKE_BINARY_DIR}/lib/transport/messages-tron.proto
+    ${CMAKE_BINARY_DIR}/lib/transport/messages-ton.proto
     ${CMAKE_BINARY_DIR}/lib/transport/messages.proto)
 
 add_custom_command(
@@ -136,6 +151,18 @@ add_custom_command(
     ${PROTOC_BINARY} -I. -I/usr/include
       --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb
       "--nanopb_out=-f messages-mayachain.options:." messages-mayachain.proto
+  COMMAND
+    ${PROTOC_BINARY} -I. -I/usr/include
+      --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb
+      "--nanopb_out=-f messages-solana.options:." messages-solana.proto
+  COMMAND
+    ${PROTOC_BINARY} -I. -I/usr/include
+      --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb
+      "--nanopb_out=-f messages-tron.options:." messages-tron.proto
+  COMMAND
+    ${PROTOC_BINARY} -I. -I/usr/include
+      --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb
+      "--nanopb_out=-f messages-ton.options:." messages-ton.proto
   COMMAND
     ${PROTOC_BINARY} -I. -I/usr/include
       --plugin=nanopb=${NANOPB_DIR}/generator/protoc-gen-nanopb

lib/transport/pb_decode.c

@@ -452,14 +452,18 @@ static bool checkreturn decode_static_field(pb_istream_t *stream,
       }
 
     case PB_HTYPE_ONEOF:
-      *(pb_size_t *)iter->pSize = iter->pos->tag;
-      if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE) {
+      if (PB_LTYPE(type) == PB_LTYPE_SUBMESSAGE &&
+                *(pb_size_t*)iter->pSize != iter->pos->tag)
+      {
         /* We memset to zero so that any callbacks are set to NULL.
-         * Then set any default values. */
+         * This is because the callbacks might otherwise have values
+         * from some other union field. */
         memset(iter->pData, 0, iter->pos->data_size);
         pb_message_set_to_defaults((const pb_field_t *)iter->pos->ptr,
                                    iter->pData);
       }
+      *(pb_size_t*)iter->pSize = iter->pos->tag;
+
       return func(stream, iter->pos, iter->pData);
 
     default:

tools/firmware/keepkey_main.c

@@ -174,8 +174,24 @@ int main(void) {
   _mmhusr_isr = (void *)&mmhisr;
 
   { // limit sigRet lifetime to this block
+    /* F5 hardening: replace full signatures_ok() (~1 sec crypto) with fast
+     * metadata presence check. The bootloader has already performed the
+     * authoritative signature verification before jumping here. We only
+     * need to know whether the bootloader considered us signed, which is
+     * indicated by valid signature indices in flash metadata. */
     int sigRet = SIG_FAIL;
-    sigRet = signatures_ok();
+
+    volatile uint8_t si1 = *((volatile uint8_t *)FLASH_META_SIGINDEX1);
+    volatile uint8_t si2 = *((volatile uint8_t *)FLASH_META_SIGINDEX2);
+    volatile uint8_t si3 = *((volatile uint8_t *)FLASH_META_SIGINDEX3);
+
+    if (si1 >= 1 && si1 <= PUBKEYS &&
+        si2 >= 1 && si2 <= PUBKEYS &&
+        si3 >= 1 && si3 <= PUBKEYS &&
+        si1 != si2 && si1 != si3 && si2 != si3) {
+      sigRet = SIG_OK;
+    }
+
     flash_collectHWEntropy(SIG_OK == sigRet);
 
     /* Drop privileges */