Code improvements / Fix warnings

- Fix CodeQL warnings
- Port lots of improvements/fixes from PR #225
- replace 'array_hexstr' and '%*H' format by sdk function 'format_hex'
- Add 'noreturn' attribute in 'main.c'

Makefile

@@ -21,8 +21,6 @@ endif
 
 include $(BOLOS_SDK)/Makefile.defines
 
-# Allows to use sprintf(..., "0x%.*H", ...)
-CFLAGS += -Wno-format-invalid-specifier -Wno-format-extra-args
 ########################################
 #        Mandatory configuration       #
 ########################################
@@ -54,6 +52,7 @@ APP_SOURCE_FILES += ./ethereum-plugin-sdk/src/common_utils.c
 APP_SOURCE_FILES += ./ethereum-plugin-sdk/src/plugin_utils.c
 INCLUDES_PATH += ./ethereum-plugin-sdk/src
 APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/crypto_helpers.c
+APP_SOURCE_FILES += ${BOLOS_SDK}/lib_standard_app/format.c
 INCLUDES_PATH += ${BOLOS_SDK}/lib_standard_app
 
 ifeq ($(TARGET_NAME),TARGET_STAX)

src/eth_plugin_handler.c

@@ -102,11 +102,11 @@ static void eth_plugin_perform_init_default(uint8_t *contractAddress,
 static bool eth_plugin_perform_init_old_internal(uint8_t *contractAddress,
                                                  ethPluginInitContract_t *init) {
     uint8_t i, j;
-    const uint8_t **selectors;
+    const uint8_t *const *selectors;
 
     // Search internal plugin list
     for (i = 0;; i++) {
-        selectors = (const uint8_t **) PIC(INTERNAL_ETH_PLUGINS[i].selectors);
+        selectors = (const uint8_t *const *) PIC(INTERNAL_ETH_PLUGINS[i].selectors);
         if (selectors == NULL) {
             break;
         }

src/eth_plugin_internal.c

@@ -25,11 +25,11 @@ const uint8_t* const ETH2_SELECTORS[NUM_ETH2_SELECTORS] = {ETH2_DEPOSIT_SELECTOR
 // All internal alias names start with 'minus'
 
 const internalEthPlugin_t INTERNAL_ETH_PLUGINS[] = {
-    {NULL, (const uint8_t**) ERC20_SELECTORS, NUM_ERC20_SELECTORS, "-erc20", erc20_plugin_call},
+    {NULL, ERC20_SELECTORS, NUM_ERC20_SELECTORS, "-erc20", erc20_plugin_call},
 
 #ifdef HAVE_ETH2
 
-    {NULL, (const uint8_t**) ETH2_SELECTORS, NUM_ETH2_SELECTORS, "-eth2", eth2_plugin_call},
+    {NULL, ETH2_SELECTORS, NUM_ETH2_SELECTORS, "-eth2", eth2_plugin_call},
 
 #endif
 

src/eth_plugin_internal.h

@@ -8,12 +8,12 @@
 void erc721_plugin_call(int message, void* parameters);
 void erc1155_plugin_call(int message, void* parameters);
 
-typedef bool (*PluginAvailableCheck)(void);
+typedef bool (*const PluginAvailableCheck)(void);
 typedef void (*PluginCall)(int, void*);
 
 typedef struct internalEthPlugin_t {
     PluginAvailableCheck availableCheck;
-    const uint8_t** selectors;
+    const uint8_t* const* selectors;
     uint8_t num_selectors;
     char alias[10];
     PluginCall impl;

src/handle_check_address.c

@@ -7,7 +7,7 @@
 
 #define ZERO(x) explicit_bzero(&x, sizeof(x))
 
-void handle_check_address(check_address_parameters_t* params, chain_config_t* chain_config) {
+void handle_check_address(check_address_parameters_t* params, const chain_config_t* chain_config) {
     params->result = 0;
     PRINTF("Params on the address %d\n", (unsigned int) params);
     PRINTF("Address to check %s\n", params->address_to_check);

src/handle_check_address.h

@@ -5,6 +5,6 @@
 #include "chainConfig.h"
 
 void handle_check_address(check_address_parameters_t* check_address_params,
-                          chain_config_t* chain_config);
+                          const chain_config_t* chain_config);
 
 #endif  // _HANDLE_CHECK_ADDRESS_H_

src/handle_swap_sign_transaction.c

@@ -14,7 +14,7 @@
 static uint8_t* G_swap_sign_return_value_address;
 
 bool copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params,
-                                 chain_config_t* config) {
+                                 const chain_config_t* config) {
     // first copy parameters to stack, and then to global data.
     // We need this "trick" as the input data position can overlap with app-ethereum globals
     txStringProperties_t stack_data;
@@ -80,7 +80,7 @@ void __attribute__((noreturn)) finalize_exchange_sign_transaction(bool is_succes
     os_lib_end();
 }
 
-void __attribute__((noreturn)) handle_swap_sign_transaction(chain_config_t* config) {
+void __attribute__((noreturn)) handle_swap_sign_transaction(const chain_config_t* config) {
 #ifdef HAVE_NBGL
     // On Stax, display a spinner at startup
     UX_INIT();

src/handle_swap_sign_transaction.h

@@ -4,8 +4,8 @@
 #include "chainConfig.h"
 
 bool copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params,
-                                 chain_config_t* config);
+                                 const chain_config_t* config);
 
-void __attribute__((noreturn)) handle_swap_sign_transaction(chain_config_t* config);
+void __attribute__((noreturn)) handle_swap_sign_transaction(const chain_config_t* config);
 
 void __attribute__((noreturn)) finalize_exchange_sign_transaction(bool is_success);

src/main.c

@@ -67,7 +67,7 @@ const internalStorage_t N_storage_real;
 #ifdef HAVE_NBGL
 caller_app_t *caller_app = NULL;
 #endif
-chain_config_t *chainConfig = NULL;
+const chain_config_t *chainConfig;
 
 void reset_app_context() {
     // PRINTF("!!RESET_APP_CONTEXT\n");
@@ -114,7 +114,7 @@ unsigned short io_exchange_al(unsigned char channel, unsigned short tx_len) {
     return 0;
 }
 
-extraInfo_t *getKnownToken(uint8_t *contractAddress) {
+extraInfo_t *getKnownToken(const uint8_t *contractAddress) {
     union extraInfo_t *currentItem = NULL;
     // Works for ERC-20 & NFT tokens since both structs in the union have the
     // contract address aligned
@@ -456,7 +456,7 @@ void app_main(void) {
 // override point, but nothing more to do
 #ifdef HAVE_BAGL
 void io_seproxyhal_display(const bagl_element_t *element) {
-    io_seproxyhal_display_default((bagl_element_t *) element);
+    io_seproxyhal_display_default(element);
 }
 #endif
 
@@ -526,7 +526,7 @@ void init_coin_config(chain_config_t *coin_config) {
     coin_config->chainId = CHAIN_ID;
 }
 
-void coin_main(libargs_t *args) {
+__attribute__((noreturn)) void coin_main(libargs_t *args) {
     chain_config_t config;
     if (args) {
         if (args->chain_config != NULL) {
@@ -612,10 +612,10 @@ void coin_main(libargs_t *args) {
         }
         END_TRY;
     }
-    app_exit();
+    os_sched_exit(-1);
 }
 
-void library_main(libargs_t *args) {
+__attribute__((noreturn)) void library_main(libargs_t *args) {
     chain_config_t coin_config;
     if (args->chain_config == NULL) {
         // We have been started directly by Exchange, not by a Clone. Init default chain

src/shared_context.h

@@ -147,7 +147,7 @@ typedef union {
     strDataTmp_t tmp;
 } strings_t;
 
-extern chain_config_t *chainConfig;
+extern const chain_config_t *chainConfig;
 
 extern tmpCtx_t tmpCtx;
 extern txContext_t txContext;

src/ui_callbacks.h

@@ -26,6 +26,6 @@ void ui_warning_contract_data(void);
 
 void io_seproxyhal_send_status(uint32_t sw);
 void finalizeParsing(bool direct);
-extraInfo_t *getKnownToken(uint8_t *contractAddress);
+extraInfo_t *getKnownToken(const uint8_t *contractAddress);
 
 #endif  // _UI_CALLBACKS_H_

src/uint128.c

@@ -53,11 +53,9 @@ void shiftl128(const uint128_t *const number, uint32_t value, uint128_t *const t
     } else if (value < 64) {
         UPPER_P(target) = (UPPER_P(number) << value) + (LOWER_P(number) >> (64 - value));
         LOWER_P(target) = (LOWER_P(number) << value);
-    } else if ((128 > value) && (value > 64)) {
+    } else {
         UPPER_P(target) = LOWER_P(number) << (value - 64);
         LOWER_P(target) = 0;
-    } else {
-        clear128(target);
     }
 }
 
@@ -74,11 +72,9 @@ void shiftr128(const uint128_t *const number, uint32_t value, uint128_t *const t
         UPPER(result) = UPPER_P(number) >> value;
         LOWER(result) = (UPPER_P(number) << (64 - value)) + (LOWER_P(number) >> value);
         copy128(target, &result);
-    } else if ((128 > value) && (value > 64)) {
+    } else {
         LOWER_P(target) = UPPER_P(number) >> (value - 64);
         UPPER_P(target) = 0;
-    } else {
-        clear128(target);
     }
 }
 

src/uint256.c

@@ -59,11 +59,9 @@ void shiftl256(const uint256_t *const number, uint32_t value, uint256_t *const t
         add128(&tmp1, &tmp2, &UPPER(result));
         shiftl128(&LOWER_P(number), value, &LOWER(result));
         copy256(target, &result);
-    } else if ((256 > value) && (value > 128)) {
+    } else {
         shiftl128(&LOWER_P(number), (value - 128), &UPPER_P(target));
         clear128(&LOWER_P(target));
-    } else {
-        clear256(target);
     }
 }
 
@@ -84,11 +82,9 @@ void shiftr256(const uint256_t *const number, uint32_t value, uint256_t *const t
         shiftl128(&UPPER_P(number), (128 - value), &tmp2);
         add128(&tmp1, &tmp2, &LOWER(result));
         copy256(target, &result);
-    } else if ((256 > value) && (value > 128)) {
+    } else {
         shiftr128(&UPPER_P(number), (value - 128), &LOWER_P(target));
         clear128(&UPPER_P(target));
-    } else {
-        clear256(target);
     }
 }
 

src/uint_common.c

@@ -58,3 +58,20 @@ void reverseString(char *const str, uint32_t length) {
         str[j] = c;
     }
 }
+
+int bytes_to_string(char *out, size_t outl, const void *value, size_t len) {
+    if (outl <= 2) {
+        // Need at least '0x' and 1 digit
+        return -1;
+    }
+    if (strlcpy(out, "0x", outl) != 2) {
+        goto err;
+    }
+    if (format_hex(value, len, out + 2, outl - 2) < 0) {
+        goto err;
+    }
+    return 0;
+err:
+    *out = '\0';
+    return -1;
+}

src/uint_common.h

@@ -21,6 +21,9 @@
 #define _UINT_COMMON_H_
 
 #include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include "format.h"
 
 #define UPPER_P(x) x->elements[0]
 #define LOWER_P(x) x->elements[1]
@@ -32,4 +35,6 @@ void read_u64_be(const uint8_t *const in, uint64_t *const out);
 uint64_t readUint64BE(const uint8_t *const buffer);
 void reverseString(char *const str, uint32_t length);
 
+int bytes_to_string(char *out, size_t outl, const void *value, size_t len);
+
 #endif  //_UINT_COMMON_H_

src_bagl/ui_flow_getEth2PublicKey.c

@@ -2,9 +2,13 @@
 
 #include "shared_context.h"
 #include "ui_callbacks.h"
+#include "uint_common.h"
 
 void prepare_eth2_public_key() {
-    snprintf(strings.tmp.tmp, 100, "0x%.*H", 48, tmpCtx.publicKeyContext.publicKey.W);
+    bytes_to_string(strings.tmp.tmp,
+                    sizeof(strings.tmp.tmp),
+                    tmpCtx.publicKeyContext.publicKey.W,
+                    48);
 }
 
 // clang-format off

src_bagl/ui_flow_signMessage712.c

@@ -57,7 +57,7 @@ UX_STEP_INIT(
 UX_STEP_CB(
     ux_712_step_approve,
     pb,
-    ui_712_approve(NULL),
+    ui_712_approve(),
     {
       &C_icon_validate_14,
       "Approve",
@@ -65,7 +65,7 @@ UX_STEP_CB(
 UX_STEP_CB(
     ux_712_step_reject,
     pb,
-    ui_712_reject(NULL),
+    ui_712_reject(),
     {
       &C_icon_crossmark,
       "Reject",

src_bagl/ui_flow_signMessage712_v0.c

@@ -1,21 +1,20 @@
 #include "shared_context.h"
 #include "ui_callbacks.h"
 #include "common_712.h"
+#include "uint_common.h"
 
 void prepare_domain_hash_v0() {
-    snprintf(strings.tmp.tmp,
+    bytes_to_string(strings.tmp.tmp,
-             sizeof(strings.tmp.tmp),
+                    sizeof(strings.tmp.tmp),
-             "0x%.*H",
+                    tmpCtx.messageSigningContext712.domainHash,
-             KECCAK256_HASH_BYTESIZE,
+                    KECCAK256_HASH_BYTESIZE);
-             tmpCtx.messageSigningContext712.domainHash);
 }
 
 void prepare_message_hash_v0() {
-    snprintf(strings.tmp.tmp,
+    bytes_to_string(strings.tmp.tmp,
-             sizeof(strings.tmp.tmp),
+                    sizeof(strings.tmp.tmp),
-             "0x%.*H",
+                    tmpCtx.messageSigningContext712.messageHash,
-             KECCAK256_HASH_BYTESIZE,
+                    KECCAK256_HASH_BYTESIZE);
-             tmpCtx.messageSigningContext712.messageHash);
 }
 
 // clang-format off
@@ -46,7 +45,7 @@ UX_STEP_NOCB_INIT(
 UX_STEP_CB(
     ux_sign_712_v0_flow_4_step,
     pbb,
-    ui_712_approve_cb(NULL),
+    ui_712_approve_cb(),
     {
       &C_icon_validate_14,
       "Sign",
@@ -55,7 +54,7 @@ UX_STEP_CB(
 UX_STEP_CB(
     ux_sign_712_v0_flow_5_step,
     pbb,
-    ui_712_reject_cb(NULL),
+    ui_712_reject_cb(),
     {
       &C_icon_crossmark,
       "Cancel",

src_features/performPrivacyOperation/cmd_performPrivacyOperation.c

@@ -3,6 +3,7 @@
 
 #include "feature_performPrivacyOperation.h"
 #include "common_ui.h"
+#include "uint_common.h"
 
 #define P2_PUBLIC_ENCRYPTION_KEY 0x00
 #define P2_SHARED_SECRET         0x01
@@ -106,11 +107,11 @@ void handlePerformPrivacyOperation(uint8_t p1,
         for (uint8_t i = 0; i < 32; i++) {
             privateKeyData[i] = tmpCtx.publicKeyContext.publicKey.W[32 - i];
         }
-        snprintf(strings.common.fullAmount,
+        format_hex(privateKeyData,
-                 sizeof(strings.common.fullAmount) - 1,
+                   32,
-                 "%.*H",
+                   strings.common.fullAmount,
-                 32,
+                   sizeof(strings.common.fullAmount) - 1);
-                 privateKeyData);
+
         if (p2 == P2_PUBLIC_ENCRYPTION_KEY) {
             ui_display_privacy_public_key();
         } else {

src_features/performPrivacyOperation/feature_performPrivacyOperation.h

@@ -1,3 +1,8 @@
+#ifndef _PERFORM_PRIVACY_OPERATION_H_
+#define _PERFORM_PRIVACY_OPERATION_H_
+
 #include "shared_context.h"
 
 uint32_t set_result_perform_privacy_operation(void);
+
+#endif  // _PERFORM_PRIVACY_OPERATION_H_

src_features/provideNFTInformation/cmd_provideNFTInfo.c

@@ -137,7 +137,7 @@ void handleProvideNFTInformation(uint8_t p1,
     offset += CHAIN_ID_SIZE;
 
     uint8_t keyId = workBuffer[offset];
-    uint8_t *rawKey;
+    const uint8_t *rawKey;
     uint8_t rawKeyLen;
 
     PRINTF("KeyID: %d\n", keyId);
@@ -146,7 +146,7 @@ void handleProvideNFTInformation(uint8_t p1,
         case STAGING_NFT_METADATA_KEY:
 #endif
         case PROD_NFT_METADATA_KEY:
-            rawKey = (uint8_t *) LEDGER_NFT_METADATA_PUBLIC_KEY;
+            rawKey = LEDGER_NFT_METADATA_PUBLIC_KEY;
             rawKeyLen = sizeof(LEDGER_NFT_METADATA_PUBLIC_KEY);
             break;
         default:

src_features/signMessageEIP712/ui_logic.c

@@ -17,6 +17,7 @@
 #include "commands_712.h"
 #include "common_ui.h"
 #include "domain_name.h"
+#include "uint_common.h"
 
 static t_ui_context *ui_ctx = NULL;
 
@@ -165,11 +166,10 @@ void ui_712_message_hash(void) {
     const char *const title = "Message hash";
 
     ui_712_set_title(title, strlen(title));
-    snprintf(strings.tmp.tmp,
+    bytes_to_string(strings.tmp.tmp,
-             sizeof(strings.tmp.tmp),
+                    sizeof(strings.tmp.tmp),
-             "0x%.*H",
+                    tmpCtx.messageSigningContext712.messageHash,
-             KECCAK256_HASH_BYTESIZE,
+                    KECCAK256_HASH_BYTESIZE);
-             tmpCtx.messageSigningContext712.messageHash);
     ui_712_redraw_generic_step();
 }
 
@@ -286,7 +286,7 @@ static bool ui_712_format_bool(const uint8_t *const data, uint8_t length) {
  */
 static void ui_712_format_bytes(const uint8_t *const data, uint8_t length) {
     if (ui_712_field_shown()) {
-        snprintf(strings.tmp.tmp, sizeof(strings.tmp.tmp), "0x%.*H", length, data);
+        bytes_to_string(strings.tmp.tmp, sizeof(strings.tmp.tmp), data, length);
         // +2 for the "0x"
         // x2 for each byte value is represented by 2 ASCII characters
         if ((2 + (length * 2)) > (sizeof(strings.tmp.tmp) - 1)) {

src_features/signTx/logic_signTx.c

@@ -9,10 +9,11 @@
 #include "ui_callbacks.h"
 #include "apdu_constants.h"
 #include "crypto_helpers.h"
+#include "format.h"
 
 #define ERR_SILENT_MODE_CHECK_FAILED 0x6001
 
-uint32_t splitBinaryParameterPart(char *result, uint8_t *parameter) {
+static uint32_t splitBinaryParameterPart(char *result, size_t result_size, uint8_t *parameter) {
     uint32_t i;
     for (i = 0; i < 8; i++) {
         if (parameter[i] != 0x00) {
@@ -25,7 +26,7 @@ uint32_t splitBinaryParameterPart(char *result, uint8_t *parameter) {
         result[2] = '\0';
         return 2;
     } else {
-        array_hexstr(result, parameter + i, 8 - i);
+        format_hex(parameter + i, 8 - i, result, result_size);
         return ((8 - i) * 2);
     }
 }
@@ -144,7 +145,10 @@ customStatus_e customProcessor(txContext_t *context) {
             }
             dataContext.tokenContext.fieldOffset = 0;
             if (fieldPos == 0) {
-                array_hexstr(strings.tmp.tmp, dataContext.tokenContext.data, 4);
+                format_hex(dataContext.tokenContext.data,
+                           4,
+                           strings.tmp.tmp,
+                           sizeof(strings.tmp.tmp));
                 ui_confirm_selector();
             } else {
                 uint32_t offset = 0;
@@ -155,6 +159,7 @@ customStatus_e customProcessor(txContext_t *context) {
                          dataContext.tokenContext.fieldIndex);
                 for (i = 0; i < 4; i++) {
                     offset += splitBinaryParameterPart(strings.tmp.tmp + offset,
+                                                       sizeof(strings.tmp.tmp) - offset,
                                                        dataContext.tokenContext.data + 8 * i);
                     if (i != 3) {
                         strings.tmp.tmp[offset++] = ':';

src_nbgl/ui_get_eth2_public_key.c

@@ -2,6 +2,7 @@
 #include "shared_context.h"
 #include "ui_callbacks.h"
 #include "ui_nbgl.h"
+#include "uint_common.h"
 
 static void reviewReject(void) {
     io_seproxyhal_touch_address_cancel(NULL);
@@ -21,7 +22,10 @@ static void reviewChoice(bool confirm) {
 }
 
 static void buildScreen(void) {
-    snprintf(strings.tmp.tmp, 100, "0x%.*H", 48, tmpCtx.publicKeyContext.publicKey.W);
+    bytes_to_string(strings.tmp.tmp,
+                    sizeof(strings.tmp.tmp),
+                    tmpCtx.publicKeyContext.publicKey.W,
+                    48);
     nbgl_useCaseAddressConfirmation(strings.tmp.tmp, reviewChoice);
 }
 

src_nbgl/ui_sign_712_v0.c

@@ -4,13 +4,14 @@
 #include "network.h"
 #include "ui_message_signing.h"
 #include "ui_signing.h"
+#include "uint_common.h"
 
 static nbgl_layoutTagValue_t pairs[2];
 
 static void start_review(void);  // forward declaration
 
 static char *format_hash(const uint8_t *hash, char *buffer, size_t buffer_size, size_t offset) {
-    snprintf(buffer + offset, buffer_size - offset, "0x%.*H", KECCAK256_HASH_BYTESIZE, hash);
+    bytes_to_string(buffer + offset, buffer_size - offset, hash, KECCAK256_HASH_BYTESIZE);
     return buffer + offset;
 }
 

src_plugins/erc1155/erc1155_plugin.c

@@ -27,7 +27,7 @@ static void handle_init_contract(void *parameters) {
     }
     uint8_t i;
     for (i = 0; i < SELECTORS_COUNT; i++) {
-        if (memcmp((uint8_t *) PIC(ERC1155_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
+        if (memcmp(PIC(ERC1155_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
             context->selectorIndex = i;
             break;
         }

src_plugins/erc20/erc20_plugin.c

@@ -64,7 +64,7 @@ const contract_t CONTRACTS[NUM_CONTRACTS] = {
 
 bool check_contract(erc20_parameters_t *context) {
     for (size_t i = 0; i < NUM_CONTRACTS; i++) {
-        contract_t *contract = (contract_t *) PIC(&CONTRACTS[i]);
+        const contract_t *contract = (const contract_t *) PIC(&CONTRACTS[i]);
         if (memcmp(contract->address, context->destinationAddress, ADDRESS_LENGTH) == 0) {
             strlcpy(context->contract_name, contract->name, sizeof(context->contract_name));
             return true;

src_plugins/erc721/erc721_plugin.c

@@ -32,7 +32,7 @@ static void handle_init_contract(void *parameters) {
     }
     uint8_t i;
     for (i = 0; i < SELECTORS_COUNT; i++) {
-        if (memcmp((uint8_t *) PIC(ERC721_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
+        if (memcmp(PIC(ERC721_SELECTORS[i]), msg->selector, SELECTOR_SIZE) == 0) {
             context->selectorIndex = i;
             break;
         }

src_plugins/eth2/eth2_plugin.c

@@ -199,7 +199,7 @@ void eth2_plugin_call(int message, void *parameters) {
             switch (msg->screenIndex) {
                 case 0: {  // Amount screen
                     uint8_t decimals = WEI_TO_ETHER;
-                    char *ticker = chainConfig->coinName;
+                    const char *ticker = chainConfig->coinName;
                     strlcpy(msg->title, "Amount", msg->titleLength);
                     if (!amountToString(tmpContent.txContent.value.value,
                                         tmpContent.txContent.value.length,