LedgerHQ · jibeee · Jul 21, 2021 · Jul 21, 2021 · Jul 21, 2021
diff --git a/.github/workflows/ci-workflow.yml b/.github/workflows/ci-workflow.yml
@@ -32,6 +32,26 @@ jobs:
           name: bitcoin-testnet-app
           path: bitcoin-testnet-bin
 
+  scan-build:
+    name: Clang Static Analyzer
+    runs-on: ubuntu-latest
+
+    container:
+      image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder:latest
+
+    steps:
+      - uses: actions/checkout@v2
+
+      - name: Build with Clang Static Analyzer
+        run: |
+          make clean
+          scan-build --use-cc=clang -analyze-headers -enable-checker security -enable-checker unix -enable-checker valist -o scan-build --status-bugs make default
+      - uses: actions/upload-artifact@v2
+        if: failure()
+        with:
+          name: scan-build
+          path: scan-build
+
   job_test:
     name: Tests
     needs: job_build

diff --git a/Makefile b/Makefile
@@ -274,7 +274,7 @@ endif
 CC       := $(CLANGPATH)clang
 
 #CFLAGS   += -O0
-CFLAGS   += -O3 -Os
+CFLAGS   += -O3 -Os -Wno-format-invalid-specifier
 AS     := $(GCCPATH)arm-none-eabi-gcc
 
 LD       := $(GCCPATH)arm-none-eabi-gcc

diff --git a/include/btchip_helpers.h b/include/btchip_helpers.h
@@ -21,7 +21,7 @@
 
 #include "os.h"
 #include "cx.h"
-#include "stdbool.h"
+#include <stdbool.h>
 
 #define OUTPUT_SCRIPT_REGULAR_PRE_LENGTH 4
 #define OUTPUT_SCRIPT_REGULAR_POST_LENGTH 2
@@ -43,7 +43,7 @@ unsigned char btchip_output_script_is_op_call(unsigned char *buffer,
 void btchip_sleep16(unsigned short delay);
 void btchip_sleep32(unsigned long int delayEach, unsigned long int delayRepeat);
 
-unsigned long int btchip_read_u32(unsigned char *buffer, unsigned char be,
+unsigned long int btchip_read_u32(const uint8_t *buffer, unsigned char be,
                                   unsigned char skipSign);
 
 void btchip_write_u32_be(unsigned char *buffer, unsigned long int value);
@@ -71,8 +71,8 @@ void btchip_private_derive_keypair(unsigned char *bip32Path,
                                    cx_ecfp_private_key_t * private_key,
                                    cx_ecfp_public_key_t* public_key);
 
-unsigned char bip44_derivation_guard(unsigned char *bip32Path, bool is_change_path);
-unsigned char enforce_bip44_coin_type(unsigned char *bip32Path, bool for_pubkey);
+bool bip44_derivation_guard(const uint8_t *bip32Path, bool is_change_path);
+bool enforce_bip44_coin_type(const uint8_t *bip32Path, bool for_pubkey);
 unsigned char bip32_print_path(unsigned char *bip32Path, char* out, unsigned char max_out_len);
 
 // void btchip_set_check_internal_structure_integrity(unsigned char

diff --git a/src/btchip.c b/src/btchip.c
@@ -48,7 +48,7 @@ void handleGetWalletId(volatile unsigned short *tx) {
   // pubkey -> sha512
   cx_hash_sha512(pub.W, sizeof(pub.W), t, sizeof(t));
   // ! cookie !
-  os_memmove(G_io_apdu_buffer, t, 64);
+  memcpy(G_io_apdu_buffer, t, 64);
   btchip_context_D.sw = 0x9000;
   *tx = 64;
 }
@@ -145,7 +145,7 @@ void app_dispatch(void) {
 }
 
 void app_main(void) {
-    os_memset(G_io_apdu_buffer, 0, 255); // paranoia
+    memset(G_io_apdu_buffer, 0, 255); // paranoia
 
     // Process the incoming APDUs
 

diff --git a/src/btchip_apdu_get_coin_version.c b/src/btchip_apdu_get_coin_version.c
@@ -37,11 +37,11 @@ unsigned short btchip_apdu_get_coin_version() {
     G_io_apdu_buffer[offset++] = G_coin_config->p2sh_version;
     G_io_apdu_buffer[offset++] = G_coin_config->family;
     G_io_apdu_buffer[offset++] = strlen(G_coin_config->coinid);
-    os_memmove(G_io_apdu_buffer + offset, G_coin_config->coinid,
+    memcpy(G_io_apdu_buffer + offset, G_coin_config->coinid,
                strlen(G_coin_config->coinid));
     offset += strlen(G_coin_config->coinid);
     G_io_apdu_buffer[offset++] = strlen(G_coin_config->name_short);
-    os_memmove(G_io_apdu_buffer + offset, G_coin_config->name_short,
+    memcpy(G_io_apdu_buffer + offset, G_coin_config->name_short,
                strlen(G_coin_config->name_short));
     offset += strlen(G_coin_config->name_short);
     btchip_context_D.outLength = offset;

diff --git a/src/btchip_apdu_get_trusted_input.c b/src/btchip_apdu_get_trusted_input.c
@@ -84,7 +84,7 @@ unsigned short btchip_apdu_get_trusted_input() {
 
         btchip_write_u32_le(G_io_apdu_buffer + 4 + 32,
                             btchip_context_D.transactionTargetInput);
-        os_memmove(G_io_apdu_buffer + 4 + 32 + 4,
+        memcpy(G_io_apdu_buffer + 4 + 32 + 4,
                    btchip_context_D.transactionContext.transactionAmount, 8);
 
         cx_hmac_sha256((uint8_t *)N_btchip.bkp.trustedinput_key,

diff --git a/src/btchip_apdu_get_wallet_public_key.c b/src/btchip_apdu_get_wallet_public_key.c
@@ -37,7 +37,7 @@ int get_public_key_chain_code(unsigned char* keyPath, bool uncompressedPublicKey
         keyLength = 33;
     }
 
-    os_memmove(publicKey, public_key.W,
+    memmove(publicKey, public_key.W,
                sizeof(public_key.W));
     return keyLength;
 }
@@ -48,7 +48,7 @@ unsigned short btchip_apdu_get_wallet_public_key() {
         ((N_btchip.bkp.config.options & BTCHIP_OPTION_UNCOMPRESSED_KEYS) != 0);
     uint32_t request_token;
     unsigned char chainCode[32];
-    uint8_t is_derivation_path_unusual;
+    bool is_derivation_path_unusual;
 
     bool display = (G_io_apdu_buffer[ISO_OFFSET_P1] == P1_DISPLAY);
     bool display_request_token = N_btchip.pubKeyRequestRestriction && (G_io_apdu_buffer[ISO_OFFSET_P1] == P1_REQUEST_TOKEN) && G_io_apdu_media == IO_APDU_MEDIA_U2F;
@@ -88,9 +88,7 @@ unsigned short btchip_apdu_get_wallet_public_key() {
     if (G_io_apdu_buffer[ISO_OFFSET_LC] < 0x01) {
         return BTCHIP_SW_INCORRECT_LENGTH;
     }
-    if (display) {
-        is_derivation_path_unusual = set_key_path_to_display(G_io_apdu_buffer + ISO_OFFSET_CDATA);
-    }
+    is_derivation_path_unusual = set_key_path_to_display(G_io_apdu_buffer + ISO_OFFSET_CDATA);
 
     if(display_request_token){
         uint8_t request_token_offset = ISO_OFFSET_CDATA + G_io_apdu_buffer[ISO_OFFSET_CDATA]*4 + 1;
@@ -113,7 +111,7 @@ unsigned short btchip_apdu_get_wallet_public_key() {
 
     PRINTF("pin ok\n");
 
-    unsigned char bip44_enforced = enforce_bip44_coin_type(G_io_apdu_buffer + ISO_OFFSET_CDATA, true);
+    bool bip44_enforced = enforce_bip44_coin_type(G_io_apdu_buffer + ISO_OFFSET_CDATA, true);
 
     G_io_apdu_buffer[0] = 65;
     keyLength = get_public_key_chain_code(G_io_apdu_buffer + ISO_OFFSET_CDATA, uncompressedPublicKeys, G_io_apdu_buffer + 1, chainCode);
@@ -166,7 +164,7 @@ unsigned short btchip_apdu_get_wallet_public_key() {
     }
 
     // output chain code
-    os_memmove(G_io_apdu_buffer + 1 + 65 + 1 + keyLength, chainCode,
+    memcpy(G_io_apdu_buffer + 1 + 65 + 1 + keyLength, chainCode,
                sizeof(chainCode));
     btchip_context_D.outLength = 1 + 65 + 1 + keyLength + sizeof(chainCode);
 
@@ -181,18 +179,18 @@ unsigned short btchip_apdu_get_wallet_public_key() {
             return BTCHIP_SW_INCORRECT_DATA;
         }
         // Hax, avoid wasting space
-        os_memmove(G_io_apdu_buffer + 200, G_io_apdu_buffer + 67, keyLength);
+        memmove(G_io_apdu_buffer + 200, G_io_apdu_buffer + 67, keyLength);
         G_io_apdu_buffer[200 + keyLength] = '\0';
         btchip_context_D.io_flags |= IO_ASYNCH_REPLY;
         btchip_bagl_display_public_key(is_derivation_path_unusual);
     }
     // If the token requested has already been approved in a previous call, the source is trusted so don't ask for approval again
     else if(display_request_token &&
-           (!btchip_context_D.has_valid_token || os_memcmp(&request_token, btchip_context_D.last_token, 4)))
+           (!btchip_context_D.has_valid_token || memcmp(&request_token, btchip_context_D.last_token, 4)))
     {
         // disable the has_valid_token flag and store the new token
         btchip_context_D.has_valid_token = false;
-        os_memcpy(btchip_context_D.last_token, &request_token, 4);
+        memcpy(btchip_context_D.last_token, &request_token, 4);
         // Hax, avoid wasting space
         snprintf((char *)G_io_apdu_buffer + 200, 9, "%08X", request_token);
         G_io_apdu_buffer[200 + 8] = '\0';

diff --git a/src/btchip_apdu_hash_input_finalize_full.c b/src/btchip_apdu_hash_input_finalize_full.c
@@ -34,7 +34,7 @@
 void btchip_apdu_hash_input_finalize_full_reset(void) {
     btchip_context_D.currentOutputOffset = 0;
     btchip_context_D.outputParsingState = BTCHIP_OUTPUT_PARSING_NUMBER_OUTPUTS;
-    os_memset(btchip_context_D.totalOutputAmount, 0,
+    memset(btchip_context_D.totalOutputAmount, 0,
               sizeof(btchip_context_D.totalOutputAmount));
     btchip_context_D.changeOutputFound = 0;
     btchip_set_check_internal_structure_integrity(1);
@@ -84,18 +84,18 @@ static bool check_output_displayable() {
                             : isP2sh ? OUTPUT_SCRIPT_P2SH_PRE_LENGTH
                                      : OUTPUT_SCRIPT_REGULAR_PRE_LENGTH);
         if (!isP2sh &&
-            os_memcmp(btchip_context_D.currentOutput + 8 + addressOffset,
+            memcmp(btchip_context_D.currentOutput + 8 + addressOffset,
                       btchip_context_D.tmpCtx.output.changeAddress,
                       20) == 0) {
             changeFound = true;
         } else if (isP2sh && btchip_context_D.usingSegwit) {
             unsigned char changeSegwit[22];
             changeSegwit[0] = 0x00;
             changeSegwit[1] = 0x14;
-            os_memmove(changeSegwit + 2,
+            memcpy(changeSegwit + 2,
                        btchip_context_D.tmpCtx.output.changeAddress, 20);
             btchip_public_key_hash160(changeSegwit, 22, changeSegwit);
-            if (os_memcmp(btchip_context_D.currentOutput + 8 + addressOffset,
+            if (memcmp(btchip_context_D.currentOutput + 8 + addressOffset,
                           changeSegwit, 20) == 0) {
                 if (G_coin_config->flags & FLAG_SEGWIT_CHANGE_SUPPORT) {
                     changeFound = true;
@@ -209,7 +209,7 @@ bool handle_output_state() {
     }
 
     if (discardSize != 0) {
-        os_memmove(btchip_context_D.currentOutput,
+        memmove(btchip_context_D.currentOutput,
                    btchip_context_D.currentOutput + discardSize,
                    btchip_context_D.currentOutputOffset - discardSize);
         btchip_context_D.currentOutputOffset -= discardSize;
@@ -292,14 +292,14 @@ unsigned short btchip_apdu_hash_input_finalize_full_internal(
                     sw = BTCHIP_SW_CONDITIONS_OF_USE_NOT_SATISFIED;
                     goto discardTransaction;
                 }
-                os_memset(transactionSummary, 0,
+                memset(transactionSummary, 0,
                           sizeof(btchip_transaction_summary_t));
                 if (G_io_apdu_buffer[ISO_OFFSET_CDATA] == 0x00) {
                     // Called with no change path, abort, should be prevented on
                     // the client side
                     goto return_OK;
                 }
-                os_memmove(transactionSummary->keyPath,
+                memcpy(transactionSummary->keyPath,
                            G_io_apdu_buffer + ISO_OFFSET_CDATA,
                            MAX_BIP32_PATH_LENGTH);
 
@@ -350,7 +350,7 @@ unsigned short btchip_apdu_hash_input_finalize_full_internal(
                     sw = BTCHIP_SW_INCORRECT_DATA;
                     goto discardTransaction;
                 }
-                os_memmove(btchip_context_D.currentOutput +
+                memcpy(btchip_context_D.currentOutput +
                                btchip_context_D.currentOutputOffset,
                            G_io_apdu_buffer + ISO_OFFSET_CDATA, apduLength);
                 btchip_context_D.currentOutputOffset += apduLength;
@@ -425,7 +425,7 @@ unsigned short btchip_apdu_hash_input_finalize_full_internal(
 
             if (btchip_context_D.transactionContext.firstSigned) {
                 if (!btchip_context_D.tmpCtx.output.changeInitialized) {
-                    os_memset(transactionSummary, 0,
+                    memset(transactionSummary, 0,
                               sizeof(btchip_transaction_summary_t));
                 }
 
@@ -452,7 +452,7 @@ unsigned short btchip_apdu_hash_input_finalize_full_internal(
             // (this is done to keep the transaction counter limit per session
             // synchronized)
             if (btchip_context_D.transactionContext.firstSigned) {
-                os_memmove(transactionSummary->authorizationHash,
+                memcpy(transactionSummary->authorizationHash,
                            authorizationHash,
                            sizeof(transactionSummary->authorizationHash));
                 goto return_OK;
@@ -489,7 +489,7 @@ unsigned short btchip_apdu_hash_input_finalize_full_internal(
                 BTCHIP_TRANSACTION_NONE;
             btchip_context_D.outLength = 0;
 
-            os_memmove(G_io_apdu_buffer, btchip_context_D.currentOutput,
+            memcpy(G_io_apdu_buffer, btchip_context_D.currentOutput,
                        btchip_context_D.currentOutputOffset);
             btchip_context_D.outLength = btchip_context_D.currentOutputOffset;
         }
@@ -544,7 +544,7 @@ unsigned char btchip_bagl_user_action(unsigned char confirming) {
         }
 
         while (btchip_context_D.remainingOutputs != 0) {
-            os_memmove(btchip_context_D.currentOutput,
+            memmove(btchip_context_D.currentOutput,
                        btchip_context_D.currentOutput +
                            btchip_context_D.discardSize,
                        btchip_context_D.currentOutputOffset -

diff --git a/src/btchip_apdu_hash_input_start.c b/src/btchip_apdu_hash_input_start.c
@@ -110,7 +110,7 @@ unsigned short btchip_apdu_hash_input_start() {
             btchip_context_D.segwitParsedOnce = 0;
             btchip_set_check_internal_structure_integrity(1);
             // Initialize for screen pairing
-            os_memset(&btchip_context_D.tmpCtx.output, 0,
+            memset(&btchip_context_D.tmpCtx.output, 0,
                       sizeof(btchip_context_D.tmpCtx.output));
             btchip_context_D.tmpCtx.output.changeAccepted = 1;
             // Reset segwitWarningSeen flag to prevent displaying the warning for each

diff --git a/src/btchip_apdu_hash_sign.c b/src/btchip_apdu_hash_sign.c
@@ -96,7 +96,7 @@ unsigned short btchip_apdu_hash_sign() {
                 CLOSE_TRY;
                 goto catch_discardTransaction;
             }
-            os_memmove(btchip_context_D.transactionSummary.keyPath,
+            memcpy(btchip_context_D.transactionSummary.keyPath,
                 G_io_apdu_buffer + ISO_OFFSET_CDATA,
                 MAX_BIP32_PATH_LENGTH);
             parameters += (4 * G_io_apdu_buffer[ISO_OFFSET_CDATA]) + 1;

diff --git a/src/btchip_apdu_setup.c b/src/btchip_apdu_setup.c
@@ -27,7 +27,7 @@ void btchip_autosetup() {
     btchip_config_t config;
     unsigned char i;
     unsigned char tmp[32];
-    os_memset(&config, 0, sizeof(btchip_config_t));
+    memset(&config, 0, sizeof(btchip_config_t));
     config.options |= BTCHIP_OPTION_DETERMINISTIC_SIGNATURE;
     config.options |= BTCHIP_OPTION_SKIP_2FA_P2SH; // TODO : remove when
                                                    // supporting multi output

diff --git a/src/btchip_apdu_sign_message.c b/src/btchip_apdu_sign_message.c
@@ -86,7 +86,7 @@ unsigned short btchip_apdu_sign_message_internal() {
                     unsigned char chunkLength;
                     unsigned char messageLength[3];
                     unsigned char messageLengthSize;
-                    os_memset(&btchip_context_D.transactionSummary, 0,
+                    memset(&btchip_context_D.transactionSummary, 0,
                               sizeof(btchip_transaction_summary_t));
                     if (G_io_apdu_buffer[offset] > MAX_BIP32_PATH) {
                         PRINTF("Invalid path\n");
@@ -98,7 +98,7 @@ unsigned short btchip_apdu_sign_message_internal() {
                         G_coin_config->p2pkh_version;
                     btchip_context_D.transactionSummary.payToScriptHashVersion =
                         G_coin_config->p2sh_version;
-                    os_memmove(
+                    memcpy(
                         btchip_context_D.transactionSummary.keyPath,
                         G_io_apdu_buffer + offset, MAX_BIP32_PATH_LENGTH);
                     offset += (4 * G_io_apdu_buffer[offset]) + 1;
@@ -217,7 +217,7 @@ unsigned short btchip_apdu_sign_message_internal() {
             sw = SW_TECHNICAL_DETAILS(0x0F);
         }
     discard : {
-        os_memset(&btchip_context_D.transactionSummary, 0,
+        memset(&btchip_context_D.transactionSummary, 0,
                   sizeof(btchip_transaction_summary_t));
     }
         FINALLY {
@@ -265,7 +265,7 @@ unsigned short btchip_compute_hash() {
             sw = SW_TECHNICAL_DETAILS(0x0F);
         }
         FINALLY {
-            os_memset(&btchip_context_D.transactionSummary, 0,
+            memset(&btchip_context_D.transactionSummary, 0,
                       sizeof(btchip_transaction_summary_t));
         }
     }

diff --git a/src/btchip_base58.c b/src/btchip_base58.c
@@ -31,7 +31,7 @@ int btchip_decode_base58(const char *in, size_t length,
   if ((length > MAX_DEC_INPUT_SIZE) || (length < 2)) {
     return -1;
   }
-  os_memmove(tmp, in, length);
+  memcpy(tmp, in, length);
   PRINTF("To decode\n%.*H\n",length,tmp);
   for (i = 0; i < length; i++) {
     if (in[i] >= sizeof(BASE58TABLE)) {
@@ -70,7 +70,7 @@ int btchip_decode_base58(const char *in, size_t length,
     return -1;
   }
 
-  os_memmove(out, buffer + j - zeroCount, length);
+  memcpy(out, buffer + j - zeroCount, length);
   PRINTF("Decoded\n%.*H\n",length,out);
   *outlen = length;
   return 0;
@@ -99,7 +99,7 @@ int btchip_encode_base58(const unsigned char *in, size_t length,
     *outlen = outputSize;
     return -1;
   }
-  os_memset(out, 0, outputSize);
+  memset(out, 0, outputSize);
   stopAt = outputSize - 1;
   for (startAt = zeroCount; startAt < length; startAt++) {
     int carry = in[startAt];
@@ -134,7 +134,7 @@ int btchip_encode_base58(const unsigned char *in, size_t length,
     for (i = *outlen - 1; (int)i >= 0; --i)
         out[i] = BASE58ALPHABET[out[i - distance]];
   }
-  os_memset(out, BASE58ALPHABET[0], zeroCount);
+  memset(out, BASE58ALPHABET[0], zeroCount);
   // PRINTF("Length encoded %d\n", i);
   // PRINTF("Encoded\n%.*H\n",i,out);
   return 0;

diff --git a/src/btchip_context.c b/src/btchip_context.c
@@ -25,12 +25,12 @@ void btchip_autosetup(void);
 void btchip_context_init() {
     PRINTF("Context init\n");
     PRINTF("Backup size %d\n", sizeof(N_btchip.bkp));
-    os_memset(&btchip_context_D, 0, sizeof(btchip_context_D));
+    memset(&btchip_context_D, 0, sizeof(btchip_context_D));
     SB_SET(btchip_context_D.halted, 0);
     btchip_context_D.called_from_swap = 0;
     btchip_context_D.currentOutputOffset = 0;
     btchip_context_D.outputParsingState = BTCHIP_OUTPUT_PARSING_NUMBER_OUTPUTS;
-    os_memset(btchip_context_D.totalOutputAmount, 0,
+    memset(btchip_context_D.totalOutputAmount, 0,
               sizeof(btchip_context_D.totalOutputAmount));
     btchip_context_D.changeOutputFound = 0;
     btchip_context_D.segwitWarningSeen = 0;