diff --git a/.github/workflows/ci-workflow.yml b/.github/workflows/ci-workflow.yml index 40eba1e9..434a6821 100644 --- 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 index 75f41022..288cf2eb 100644 --- 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 index a99504b7..bb7f7f0d 100644 --- a/include/btchip_helpers.h +++ b/include/btchip_helpers.h @@ -21,7 +21,7 @@ #include "os.h" #include "cx.h" -#include "stdbool.h" +#include #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 index 9b40a012..08f0db83 100644 --- 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 index 4da85742..7200c5c4 100644 --- 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 index cc46f4be..d4501296 100644 --- 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 index 6ad50418..bae10b17 100644 --- 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 index 5831b25d..6ca5481e 100644 --- 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,7 +84,7 @@ 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; @@ -92,10 +92,10 @@ static bool check_output_displayable() { 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 index a2e7bfb9..d7119c55 100644 --- 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 index a9c86d03..0d108930 100644 --- 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 index 8d51e933..f7d8db87 100644 --- 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 index 2c28376a..08529fa5 100644 --- 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 index 8f89417f..af3cf96a 100644 --- 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 index fc782dfa..0aeec435 100644 --- 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; diff --git a/src/btchip_helpers.c b/src/btchip_helpers.c index a7131b0c..39a1535a 100644 --- a/src/btchip_helpers.c +++ b/src/btchip_helpers.c @@ -60,25 +60,25 @@ const unsigned char ZEN_OUTPUT_SCRIPT_POST[] = { unsigned char btchip_output_script_is_regular(unsigned char *buffer) { if (G_coin_config->native_segwit_prefix) { - if ((os_memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WPKH_PRE, + if ((memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WPKH_PRE, sizeof(TRANSACTION_OUTPUT_SCRIPT_P2WPKH_PRE)) == 0) || - (os_memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WSH_PRE, + (memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WSH_PRE, sizeof(TRANSACTION_OUTPUT_SCRIPT_P2WSH_PRE)) == 0)) { return 1; } } if (G_coin_config->kind == COIN_KIND_HORIZEN) { - if ((os_memcmp(buffer, ZEN_OUTPUT_SCRIPT_PRE, + if ((memcmp(buffer, ZEN_OUTPUT_SCRIPT_PRE, sizeof(ZEN_OUTPUT_SCRIPT_PRE)) == 0) && - (os_memcmp(buffer + sizeof(ZEN_OUTPUT_SCRIPT_PRE) + 20, + (memcmp(buffer + sizeof(ZEN_OUTPUT_SCRIPT_PRE) + 20, ZEN_OUTPUT_SCRIPT_POST, sizeof(ZEN_OUTPUT_SCRIPT_POST)) == 0)) { return 1; } } else { - if ((os_memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_PRE, + if ((memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_PRE, sizeof(TRANSACTION_OUTPUT_SCRIPT_PRE)) == 0) && - (os_memcmp(buffer + sizeof(TRANSACTION_OUTPUT_SCRIPT_PRE) + 20, + (memcmp(buffer + sizeof(TRANSACTION_OUTPUT_SCRIPT_PRE) + 20, TRANSACTION_OUTPUT_SCRIPT_POST, sizeof(TRANSACTION_OUTPUT_SCRIPT_POST)) == 0)) { return 1; @@ -89,17 +89,17 @@ unsigned char btchip_output_script_is_regular(unsigned char *buffer) { unsigned char btchip_output_script_is_p2sh(unsigned char *buffer) { if (G_coin_config->kind == COIN_KIND_HORIZEN) { - if ((os_memcmp(buffer, ZEN_TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE, + if ((memcmp(buffer, ZEN_TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE, sizeof(ZEN_TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE)) == 0) && - (os_memcmp(buffer + sizeof(ZEN_TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE) + 20, + (memcmp(buffer + sizeof(ZEN_TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE) + 20, ZEN_TRANSACTION_OUTPUT_SCRIPT_P2SH_POST, sizeof(ZEN_TRANSACTION_OUTPUT_SCRIPT_P2SH_POST)) == 0)) { return 1; } } else { - if ((os_memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE, + if ((memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE, sizeof(TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE)) == 0) && - (os_memcmp(buffer + sizeof(TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE) + 20, + (memcmp(buffer + sizeof(TRANSACTION_OUTPUT_SCRIPT_P2SH_PRE) + 20, TRANSACTION_OUTPUT_SCRIPT_P2SH_POST, sizeof(TRANSACTION_OUTPUT_SCRIPT_P2SH_POST)) == 0)) { return 1; @@ -110,9 +110,9 @@ unsigned char btchip_output_script_is_p2sh(unsigned char *buffer) { unsigned char btchip_output_script_is_native_witness(unsigned char *buffer) { if (G_coin_config->native_segwit_prefix) { - if ((os_memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WPKH_PRE, + if ((memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WPKH_PRE, sizeof(TRANSACTION_OUTPUT_SCRIPT_P2WPKH_PRE)) == 0) || - (os_memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WSH_PRE, + (memcmp(buffer, TRANSACTION_OUTPUT_SCRIPT_P2WSH_PRE, sizeof(TRANSACTION_OUTPUT_SCRIPT_P2WSH_PRE)) == 0)) { return 1; } @@ -171,7 +171,7 @@ unsigned char btchip_secure_memcmp(const void *buf1, const void *buf2, return error; } -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) { unsigned char i; unsigned long int result = 0; @@ -222,7 +222,7 @@ void btchip_compute_checksum(unsigned char* in, unsigned short inlen, unsigned c cx_hash_sha256(checksumBuffer, 32, checksumBuffer, 32); PRINTF("Checksum\n%.*H\n",4,checksumBuffer); - os_memmove(output, checksumBuffer, 4); + memcpy(output, checksumBuffer, 4); } unsigned short btchip_public_key_to_encoded_base58( @@ -245,7 +245,7 @@ unsigned short btchip_public_key_to_encoded_base58( tmpBuffer[0] = version; } } else { - os_memmove(tmpBuffer, in, 20 + versionSize); + memcpy(tmpBuffer, in, 20 + versionSize); } btchip_compute_checksum(tmpBuffer, 20 + versionSize, tmpBuffer + 20 + versionSize); @@ -283,7 +283,7 @@ unsigned short btchip_decode_base58_address(unsigned char *in, cx_sha256_init(&hash); cx_hash(&hash.header, CX_LAST, hashBuffer, 32, hashBuffer, 32); - if (os_memcmp(out + outlen - 4, hashBuffer, 4)) { + if (memcmp(out + outlen - 4, hashBuffer, 4)) { PRINTF("Hash checksum mismatch\n%.*H\n",sizeof(hashBuffer),hashBuffer); THROW(INVALID_CHECKSUM); } @@ -328,14 +328,14 @@ void btchip_private_derive_keypair(unsigned char *bip32Path, io_seproxyhal_io_heartbeat(); - os_memset(u.privateComponent, 0, sizeof(u.privateComponent)); + explicit_bzero(u.privateComponent, sizeof(u.privateComponent)); } /* Checks if the values of a derivation path are within "normal" (arbitrary) ranges: Account < 100, change == 1 or 0, address index < 50000 -Returns 1 if the path is unusual, or not compliant with BIP44*/ -unsigned char bip44_derivation_guard(unsigned char *bip32Path, bool is_change_path) { +Returns true if the path is unusual, or not compliant with BIP44*/ +bool bip44_derivation_guard(const uint8_t *bip32Path, bool is_change_path) { unsigned char i, path_len; unsigned int bip32PathInt[MAX_BIP32_PATH]; @@ -356,36 +356,36 @@ unsigned char bip44_derivation_guard(unsigned char *bip32Path, bool is_change_pa ((bip32PathInt[BIP44_PURPOSE_OFFSET]^0x80000000) != 44 && (bip32PathInt[BIP44_PURPOSE_OFFSET]^0x80000000) != 49 && (bip32PathInt[BIP44_PURPOSE_OFFSET]^0x80000000) != 84)) { - return 1; + return true; } // If the coin type doesn't match, return a warning if ((G_coin_config->bip44_coin_type != 0) && (((bip32PathInt[BIP44_COIN_TYPE_OFFSET]^0x80000000) != G_coin_config->bip44_coin_type) && ((bip32PathInt[BIP44_COIN_TYPE_OFFSET]^0x80000000) != G_coin_config->bip44_coin_type2))) { - return 1; + return true; } // If the account or address index is very high or if the change isn't 1, return a warning if((bip32PathInt[BIP44_ACCOUNT_OFFSET]^0x80000000) > MAX_BIP44_ACCOUNT_RECOMMENDED || bip32PathInt[BIP44_CHANGE_OFFSET] != is_change_path?1:0 || bip32PathInt[BIP44_ADDRESS_INDEX_OFFSET] > MAX_BIP44_ADDRESS_INDEX_RECOMMENDED) { - return 1; + return true; } - return 0; + return false; } /* Only enforce the structure or coin type for consumed UTXOs or a public address -Returns 0 if the path is non compliant, or 1 if compliant +Returns false if the path is non compliant, or true if compliant */ -unsigned char enforce_bip44_coin_type(unsigned char *bip32Path, bool for_pubkey) { +bool enforce_bip44_coin_type(const uint8_t *bip32Path, bool for_pubkey) { unsigned char i, path_len; unsigned int bip32PathInt[MAX_BIP32_PATH]; // No enforcement required if (G_coin_config->bip44_coin_type == 0) { - return 1; + return true; } // Path is too short - always require a user validation if signing if (bip32Path[0] < 2) { @@ -413,10 +413,10 @@ unsigned char enforce_bip44_coin_type(unsigned char *bip32Path, bool for_pubkey) if (((bip32PathInt[BIP44_COIN_TYPE_OFFSET]^0x80000000) == G_coin_config->bip44_coin_type) || ((bip32PathInt[BIP44_COIN_TYPE_OFFSET]^0x80000000) == G_coin_config->bip44_coin_type2)) { // Valid BIP 44 path - return 1; + return true; } // Everything else needs a user validation - return 0; + return false; } // Print a BIP32 path as an ascii string to display on the device screen @@ -471,11 +471,11 @@ void btchip_transaction_add_output(unsigned char *hash160Address, btchip_swap_bytes(btchip_context_D.tmp, amount, 8); btchip_context_D.tmp += 8; } - os_memmove(btchip_context_D.tmp, (void *)pre, sizePre); + memcpy(btchip_context_D.tmp, pre, sizePre); btchip_context_D.tmp += sizePre; - os_memmove(btchip_context_D.tmp, hash160Address, 20); + memcpy(btchip_context_D.tmp, hash160Address, 20); btchip_context_D.tmp += 20; - os_memmove(btchip_context_D.tmp, (void *)post, sizePost); + memcpy(btchip_context_D.tmp, post, sizePost); btchip_context_D.tmp += sizePost; } diff --git a/src/btchip_transaction.c b/src/btchip_transaction.c index 4f218534..9f367182 100644 --- a/src/btchip_transaction.c +++ b/src/btchip_transaction.c @@ -157,7 +157,7 @@ void transaction_parse(unsigned char parseMode) { btchip_context_D.transactionContext .transactionCurrentInputOutput = 0; btchip_context_D.transactionContext.scriptRemaining = 0; - os_memset( + memset( btchip_context_D.transactionContext.transactionAmount, 0, sizeof(btchip_context_D.transactionContext .transactionAmount)); @@ -166,7 +166,7 @@ void transaction_parse(unsigned char parseMode) { if (btchip_context_D.usingOverwinter) { if (btchip_context_D.segwitParsedOnce) { uint8_t parameters[16]; - os_memmove(parameters, OVERWINTER_PARAM_SIGHASH, 16); + memcpy(parameters, OVERWINTER_PARAM_SIGHASH, 16); if (G_coin_config->kind == COIN_KIND_ZCLASSIC) { btchip_write_u32_le(parameters + 12, CONSENSUS_BRANCH_ID_ZCLASSIC); } @@ -214,7 +214,7 @@ void transaction_parse(unsigned char parseMode) { cx_hash(&btchip_context_D.transactionHashFull.blake2b.header, 0, btchip_context_D.nExpiryHeight, sizeof(btchip_context_D.nExpiryHeight), NULL, 0); if (btchip_context_D.usingOverwinter == ZCASH_USING_OVERWINTER_SAPLING) { unsigned char valueBalance[8]; - os_memset(valueBalance, 0, sizeof(valueBalance)); + memset(valueBalance, 0, sizeof(valueBalance)); cx_hash(&btchip_context_D.transactionHashFull.blake2b.header, 0, valueBalance, sizeof(valueBalance), NULL, 0); // sapling valueBalance } cx_hash(&btchip_context_D.transactionHashFull.blake2b.header, 0, btchip_context_D.sigHashType, sizeof(btchip_context_D.sigHashType), NULL, 0); @@ -254,7 +254,7 @@ void transaction_parse(unsigned char parseMode) { // Parse the beginning of the transaction // Version check_transaction_available(4); - os_memmove(btchip_context_D.transactionVersion, + memcpy(btchip_context_D.transactionVersion, btchip_context_D.transactionBufferPointer, 4); transaction_offset_increase(4); @@ -262,7 +262,7 @@ void transaction_parse(unsigned char parseMode) { TRUSTED_INPUT_OVERWINTER) { // nVersionGroupId check_transaction_available(4); - os_memmove(btchip_context_D.nVersionGroupId, + memcpy(btchip_context_D.nVersionGroupId, btchip_context_D.transactionBufferPointer, 4); transaction_offset_increase(4); } @@ -447,7 +447,7 @@ void transaction_parse(unsigned char parseMode) { transaction_offset_increase(36); btchip_context_D.transactionHashOption = 0; check_transaction_available(8); // save amount - os_memmove( + memcpy( btchip_context_D.inputValue, btchip_context_D.transactionBufferPointer, 8); @@ -489,7 +489,7 @@ void transaction_parse(unsigned char parseMode) { } // Handle non-segwit TrustedInput (i.e. InputHashStart 1st APDU's P2==00 & data[0]==0x01) else if (trustedInputFlag && !btchip_context_D.usingSegwit) { - os_memmove( + memcpy( trustedInput, btchip_context_D.transactionBufferPointer + 2, trustedInputLength - 8); @@ -699,10 +699,10 @@ void transaction_parse(unsigned char parseMode) { sizeof(hashedSequence), hashedSequence, 32); } - os_memmove( + memcpy( btchip_context_D.segwit.cache.hashedPrevouts, hashedPrevouts, sizeof(hashedPrevouts)); - os_memmove( + memcpy( btchip_context_D.segwit.cache.hashedSequence, hashedSequence, sizeof(hashedSequence)); PRINTF("hashPrevout\n%.*H\n",32,btchip_context_D.segwit.cache.hashedPrevouts); @@ -773,7 +773,7 @@ void transaction_parse(unsigned char parseMode) { .transactionCurrentInputOutput == btchip_context_D.transactionTargetInput)) { // Save the amount - os_memmove(btchip_context_D.transactionContext + memcpy(btchip_context_D.transactionContext .transactionAmount, btchip_context_D.transactionBufferPointer, 8); diff --git a/src/cashaddr.c b/src/cashaddr.c index 45a95706..7629553f 100644 --- a/src/cashaddr.c +++ b/src/cashaddr.c @@ -117,7 +117,7 @@ int cashaddr_encode(uint8_t *hash, const size_t hash_length, uint8_t *addr, } tmp[0] = version_byte; - os_memmove(tmp + 1, hash, hash_length); + memcpy(tmp + 1, hash, hash_length); convert_bits(payload, &payload_length, 5, tmp, hash_length + 1, 8, 1); create_checksum(payload, payload_length, diff --git a/src/handle_check_address.c b/src/handle_check_address.c index df4f2e59..4225d8fd 100644 --- a/src/handle_check_address.c +++ b/src/handle_check_address.c @@ -24,7 +24,7 @@ bool derive_private_key(unsigned char* serialized_path, unsigned char serialized bool derive_compressed_public_key( unsigned char* serialized_path, unsigned char serialized_path_length, - unsigned char* public_key, unsigned char public_key_length) { + unsigned char* public_key, unsigned char public_key_length __attribute__((unused))) { cx_ecfp_private_key_t privKey; if (!derive_private_key(serialized_path, serialized_path_length, &privKey)) return false; @@ -32,7 +32,7 @@ bool derive_compressed_public_key( cx_ecfp_generate_pair(BTCHIP_CURVE, &pubKey, &privKey, 1); btchip_compress_public_key_value(pubKey.W); - os_memcpy(public_key, pubKey.W, 33); + memcpy(public_key, pubKey.W, 33); return true; } diff --git a/src/handle_get_printable_amount.c b/src/handle_get_printable_amount.c index 2bc5a807..d8e67914 100644 --- a/src/handle_get_printable_amount.c +++ b/src/handle_get_printable_amount.c @@ -9,10 +9,10 @@ int handle_get_printable_amount( get_printable_amount_parameters_t* params, btch return 0; } unsigned char amount[8]; - os_memset(amount, 0, 8); - os_memcpy(amount + (8 - params->amount_length), params->amount, params->amount_length); + memset(amount, 0, 8); + memcpy(amount + (8 - params->amount_length), params->amount, params->amount_length); unsigned char coin_name_length = strlen(config->name_short); - os_memmove(params->printable_amount, config->name_short, coin_name_length); + memcpy(params->printable_amount, config->name_short, coin_name_length); params->printable_amount[coin_name_length] = ' '; int res_length = btchip_convert_hex_amount_to_displayable_no_globals(amount, config->flags, (uint8_t *)params->printable_amount + coin_name_length + 1); params->printable_amount[res_length + coin_name_length + 1] = '\0'; diff --git a/src/main.c b/src/main.c index e8ae7f50..354714fa 100644 --- a/src/main.c +++ b/src/main.c @@ -49,6 +49,8 @@ ux_state_t G_ux; bolos_ux_params_t G_ux_params; unsigned int io_seproxyhal_touch_verify_cancel(const bagl_element_t *e) { + UNUSED(e); + // user denied the transaction, tell the USB side if (!btchip_bagl_user_action(0)) { // redraw ui @@ -58,6 +60,8 @@ unsigned int io_seproxyhal_touch_verify_cancel(const bagl_element_t *e) { } unsigned int io_seproxyhal_touch_verify_ok(const bagl_element_t *e) { + UNUSED(e); + // user accepted the transaction, tell the USB side if (!btchip_bagl_user_action(1)) { // redraw ui @@ -68,6 +72,8 @@ unsigned int io_seproxyhal_touch_verify_ok(const bagl_element_t *e) { unsigned int io_seproxyhal_touch_message_signature_verify_cancel(const bagl_element_t *e) { + UNUSED(e); + // user denied the transaction, tell the USB side btchip_bagl_user_action_message_signing(0); // redraw ui @@ -77,6 +83,8 @@ io_seproxyhal_touch_message_signature_verify_cancel(const bagl_element_t *e) { unsigned int io_seproxyhal_touch_message_signature_verify_ok(const bagl_element_t *e) { + UNUSED(e); + // user accepted the transaction, tell the USB side btchip_bagl_user_action_message_signing(1); // redraw ui @@ -85,6 +93,8 @@ io_seproxyhal_touch_message_signature_verify_ok(const bagl_element_t *e) { } unsigned int io_seproxyhal_touch_display_cancel(const bagl_element_t *e) { + UNUSED(e); + // user denied the transaction, tell the USB side btchip_bagl_user_action_display(0); // redraw ui @@ -93,6 +103,8 @@ unsigned int io_seproxyhal_touch_display_cancel(const bagl_element_t *e) { } unsigned int io_seproxyhal_touch_display_ok(const bagl_element_t *e) { + UNUSED(e); + // user accepted the transaction, tell the USB side btchip_bagl_user_action_display(1); // redraw ui @@ -101,6 +113,8 @@ unsigned int io_seproxyhal_touch_display_ok(const bagl_element_t *e) { } unsigned int io_seproxyhal_touch_sign_cancel(const bagl_element_t *e) { + UNUSED(e); + // user denied the transaction, tell the USB side btchip_bagl_user_action_signtx(0, 0); // redraw ui @@ -109,6 +123,8 @@ unsigned int io_seproxyhal_touch_sign_cancel(const bagl_element_t *e) { } unsigned int io_seproxyhal_touch_sign_ok(const bagl_element_t *e) { + UNUSED(e); + // user accepted the transaction, tell the USB side btchip_bagl_user_action_signtx(1, 0); // redraw ui @@ -116,8 +132,9 @@ unsigned int io_seproxyhal_touch_sign_ok(const bagl_element_t *e) { return 0; // DO NOT REDRAW THE BUTTON } - unsigned int io_seproxyhal_touch_display_token_cancel(const bagl_element_t *e) { + UNUSED(e); + // revoke previous valid token if there was one btchip_context_D.has_valid_token = false; // user denied the token, tell the USB side @@ -128,6 +145,8 @@ unsigned int io_seproxyhal_touch_display_token_cancel(const bagl_element_t *e) { } unsigned int io_seproxyhal_touch_display_token_ok(const bagl_element_t *e) { + UNUSED(e); + // Set the valid token flag btchip_context_D.has_valid_token = true; // user approved the token, tell the USB side @@ -725,6 +744,8 @@ unsigned short io_exchange_al(unsigned char channel, unsigned short tx_len) { } unsigned char io_event(unsigned char channel) { + UNUSED(channel); + // nothing done with the event, throw an error on the transport layer if // needed @@ -802,8 +823,7 @@ uint8_t check_fee_swap() { uint8_t prepare_fees() { if (btchip_context_D.transactionContext.relaxed) { - os_memmove(vars.tmp.feesAmount, "UNKNOWN", 7); - vars.tmp.feesAmount[7] = '\0'; + strcpy(vars.tmp.feesAmount, "UNKNOWN"); } else { unsigned char fees[8]; unsigned short textSize; @@ -813,15 +833,14 @@ uint8_t prepare_fees() { fees, btchip_context_D.transactionContext.transactionAmount, btchip_context_D.totalOutputAmount); if (borrow && G_coin_config->kind == COIN_KIND_KOMODO) { - os_memmove(vars.tmp.feesAmount, "REWARD", 6); - vars.tmp.feesAmount[6] = '\0'; + strcpy(vars.tmp.feesAmount, "REWARD"); } else { if (borrow) { PRINTF("Error : Fees not consistent"); goto error; } - os_memmove(vars.tmp.feesAmount, G_coin_config->name_short, + memcpy(vars.tmp.feesAmount, G_coin_config->name_short, strlen(G_coin_config->name_short)); vars.tmp.feesAmount[strlen(G_coin_config->name_short)] = ' '; btchip_context_D.tmp = @@ -843,17 +862,17 @@ uint8_t prepare_fees() { void get_address_from_output_script(unsigned char* script, int script_size, char* out, int out_size) { if (btchip_output_script_is_op_return(script)) { - strcpy(out, "OP_RETURN"); + strlcpy(out, "OP_RETURN", out_size); return; } if ((G_coin_config->kind == COIN_KIND_QTUM) && btchip_output_script_is_op_create(script, script_size)) { - strcpy(out, "OP_CREATE"); + strlcpy(out, "OP_CREATE", out_size); return; } if ((G_coin_config->kind == COIN_KIND_QTUM) && btchip_output_script_is_op_call(script, script_size)) { - strcpy(out, "OP_CALL"); + strlcpy(out, "OP_CALL", out_size); return; } if (btchip_output_script_is_native_witness(script)) { @@ -884,7 +903,7 @@ void get_address_from_output_script(unsigned char* script, int script_size, char versionSize = 1; address[0] = version; } - os_memmove(address + versionSize, script + addressOffset, 20); + memcpy(address + versionSize, script + addressOffset, 20); // Prepare address if (btchip_context_D.usingCashAddr) { @@ -918,8 +937,8 @@ uint8_t prepare_single_output() { // Handle Omni simple send if ((btchip_context_D.currentOutput[offset + 2] == 0x14) && - (os_memcmp(btchip_context_D.currentOutput + offset + 3, "omni", 4) == 0) && - (os_memcmp(btchip_context_D.currentOutput + offset + 3 + 4, "\0\0\0\0", 4) == 0)) { + (memcmp(btchip_context_D.currentOutput + offset + 3, "omni", 4) == 0) && + (memcmp(btchip_context_D.currentOutput + offset + 3 + 4, "\0\0\0\0", 4) == 0)) { uint8_t headerLength; uint32_t omniAssetId = btchip_read_u32(btchip_context_D.currentOutput + offset + 3 + 4 + 4, 1, 0); switch(omniAssetId) { @@ -942,7 +961,7 @@ uint8_t prepare_single_output() { vars.tmp.fullAmount[textSize + headerLength] = '\0'; } else { - os_memmove(vars.tmp.fullAmount, G_coin_config->name_short, + memcpy(vars.tmp.fullAmount, G_coin_config->name_short, strlen(G_coin_config->name_short)); vars.tmp.fullAmount[strlen(G_coin_config->name_short)] = ' '; btchip_context_D.tmp = @@ -1003,7 +1022,7 @@ unsigned int btchip_silent_confirm_single_output() { break; } - os_memmove(btchip_context_D.currentOutput, + memmove(btchip_context_D.currentOutput, btchip_context_D.currentOutput + btchip_context_D.discardSize, btchip_context_D.currentOutputOffset - @@ -1101,7 +1120,7 @@ uint8_t set_key_path_to_display(unsigned char* keyPath) { void btchip_bagl_display_public_key(uint8_t is_derivation_path_unusual) { // append a white space at the end of the address to avoid glitch on nano S - strcat((char *)G_io_apdu_buffer + 200, " "); + strlcat((char *)G_io_apdu_buffer + 200, " ", sizeof(G_io_apdu_buffer) - 200); ux_flow_init(0, is_derivation_path_unusual?ux_display_public_with_warning_flow:ux_display_public_flow, NULL); } @@ -1147,7 +1166,7 @@ void app_exit(void) { } void init_coin_config(btchip_altcoin_config_t *coin_config) { - os_memset(coin_config, 0, sizeof(btchip_altcoin_config_t)); + memset(coin_config, 0, sizeof(btchip_altcoin_config_t)); coin_config->bip44_coin_type = BIP44_COIN_TYPE; coin_config->bip44_coin_type2 = BIP44_COIN_TYPE_2; coin_config->p2pkh_version = COIN_P2PKH_VERSION;