Removed compilation warnings & now treating warnings as errors in bot…

…h UI and Signer

- Patched warning-producing SDK files in Docker ledger image build
- Fixed UI and Signer warnings
- Added -Wall build flag to UI and Signer

docker/ledger/Dockerfile

@@ -42,6 +42,16 @@ COPY os.h.patch nanos-secure-sdk-nanos-1314/include/os.h.patch
 RUN patch nanos-secure-sdk-nanos-1314/include/os.h nanos-secure-sdk-nanos-1314/include/os.h.patch
 RUN rm -f nanos-secure-sdk-nanos-1314/include/os.h.patch
 
+# Patched version of usbd_conf.h to prevent compiler warnings
+COPY usbd_conf.h.patch nanos-secure-sdk-nanos-1314/lib_stusb/usbd_conf.h.patch
+RUN patch nanos-secure-sdk-nanos-1314/lib_stusb/usbd_conf.h nanos-secure-sdk-nanos-1314/lib_stusb/usbd_conf.h.patch
+RUN rm -f nanos-secure-sdk-nanos-1314/lib_stusb/usbd_conf.h.patch
+
+# Patched version of os_io_seproxyhal.h to prevent compiler warnings
+COPY os_io_seproxyhal.h.patch nanos-secure-sdk-nanos-1314/include/os_io_seproxyhal.h.patch
+RUN patch nanos-secure-sdk-nanos-1314/include/os_io_seproxyhal.h nanos-secure-sdk-nanos-1314/include/os_io_seproxyhal.h.patch
+RUN rm -f nanos-secure-sdk-nanos-1314/include/os_io_seproxyhal.h.patch
+
 ENV BOLOS_SDK=/opt/nanos-secure-sdk-nanos-1314
 ENV CLANGPATH=/usr/bin/
 ENV GCCPATH=/opt/gcc-arm-none-eabi-9-2020-q2-update/bin/

docker/ledger/os_io_seproxyhal.h.patch

@@ -0,0 +1,3 @@
+428,429d427
+<             unsigned int UX_ALLOWED = (ux.params.len != BOLOS_UX_IGNORE &&     \
+<                                        ux.params.len != BOLOS_UX_CONTINUE);    \

docker/ledger/usbd_conf.h.patch

@@ -0,0 +1,13 @@
+45,48c45,51
+< typedef signed char int8_t;
+< typedef unsigned char uint8_t;
+< typedef unsigned short uint16_t;
+< typedef signed short int16_t;
+---
+> // NOTE: these are already defined in <stdint> for the specific container
+> // this is running on, so commenting these out prevents compiler warnings
+> //
+> // typedef signed char int8_t;
+> // typedef unsigned char uint8_t;
+> // typedef unsigned short uint16_t;
+> // typedef signed short int16_t;

ledger/src/signer/Makefile

@@ -63,6 +63,7 @@ DEFINES += HAVE_IO_USB HAVE_L4_USBLIB IO_USB_MAX_ENDPOINTS=7 IO_HID_EP_LENGTH=64
 # Compiler, assembler, and linker
 CC := $(CLANGPATH)clang
 CFLAGS += -O0 -I$(GCC_INCLUDE)
+CFLAGS += -Werror
 
 # Convert min required difficulty to what the compiler expects
 ifneq ($(TARGET_DIFFICULTY),)

ledger/src/signer/src/sign.c

@@ -56,34 +56,42 @@ int do_pubkey(unsigned int* path,
     BEGIN_TRY {
         TRY {
             // Derive and init private key
-            os_perso_derive_node_bip32(
-                CX_CURVE_256K1, path, path_length, private_key_data, NULL);
-            cx_ecdsa_init_private_key(
-                CX_CURVE_256K1, private_key_data, KEY_LEN, &private_key);
+            os_perso_derive_node_bip32(CX_CURVE_256K1,
+                                       path,
+                                       path_length,
+                                       (unsigned char*)private_key_data,
+                                       NULL);
+            cx_ecdsa_init_private_key(CX_CURVE_256K1,
+                                      (unsigned char*)private_key_data,
+                                      KEY_LEN,
+                                      (cx_ecfp_private_key_t*)&private_key);
             // Cleanup private key data
-            explicit_bzero(private_key_data, sizeof(private_key_data));
+            explicit_bzero((void*)private_key_data, sizeof(private_key_data));
             // Derive public key
-            cx_ecfp_generate_pair(CX_CURVE_256K1, &public_key, &private_key, 1);
+            cx_ecfp_generate_pair(CX_CURVE_256K1,
+                                  (cx_ecfp_public_key_t*)&public_key,
+                                  (cx_ecfp_private_key_t*)&private_key,
+                                  1);
             // Cleanup private key
-            explicit_bzero(&private_key, sizeof(private_key));
+            explicit_bzero((void*)&private_key, sizeof(private_key));
             // Output the public key
             pubkey_size = public_key.W_len;
             SAFE_MEMMOVE(dest,
                          dest_size,
                          MEMMOVE_ZERO_OFFSET,
-                         public_key.W,
+                         (void*)public_key.W,
                          public_key.W_len,
                          MEMMOVE_ZERO_OFFSET,
                          public_key.W_len,
                          { pubkey_size = DO_PUBKEY_ERROR; })
             // Cleanup public key
-            explicit_bzero(&public_key, sizeof(public_key));
+            explicit_bzero((void*)&public_key, sizeof(public_key));
         }
         CATCH_OTHER(e) {
             // Cleanup key data and fail
-            explicit_bzero(private_key_data, sizeof(private_key_data));
-            explicit_bzero(&private_key, sizeof(private_key));
-            explicit_bzero(&public_key, sizeof(public_key));
+            explicit_bzero((void*)private_key_data, sizeof(private_key_data));
+            explicit_bzero((void*)&private_key, sizeof(private_key));
+            explicit_bzero((void*)&public_key, sizeof(public_key));
             // Signal error deriving public key
             pubkey_size = DO_PUBKEY_ERROR;
         }
@@ -128,25 +136,30 @@ int do_sign(unsigned int* path,
     BEGIN_TRY {
         TRY {
             // Derive and init private key
-            os_perso_derive_node_bip32(
-                CX_CURVE_256K1, path, path_length, private_key_data, NULL);
-            cx_ecdsa_init_private_key(
-                CX_CURVE_256K1, private_key_data, KEY_LEN, &private_key);
+            os_perso_derive_node_bip32(CX_CURVE_256K1,
+                                       path,
+                                       path_length,
+                                       (unsigned char*)private_key_data,
+                                       NULL);
+            cx_ecdsa_init_private_key(CX_CURVE_256K1,
+                                      (unsigned char*)private_key_data,
+                                      KEY_LEN,
+                                      (cx_ecfp_private_key_t*)&private_key);
             // Cleanup private key data
-            explicit_bzero(private_key_data, sizeof(private_key_data));
+            explicit_bzero((void*)private_key_data, sizeof(private_key_data));
             sig_size = cx_ecdsa_sign((void*)&private_key,
                                      CX_RND_RFC6979 | CX_LAST,
                                      CX_SHA256,
                                      message,
                                      message_size,
                                      dest);
             // Cleanup private key
-            explicit_bzero(&private_key, sizeof(private_key));
+            explicit_bzero((void*)&private_key, sizeof(private_key));
         }
         CATCH_OTHER(e) {
             // Cleanup key data and fail
-            explicit_bzero(private_key_data, sizeof(private_key_data));
-            explicit_bzero(&private_key, sizeof(private_key));
+            explicit_bzero((void*)private_key_data, sizeof(private_key_data));
+            explicit_bzero((void*)&private_key, sizeof(private_key));
             // Signal error signing
             sig_size = DO_SIGN_ERROR;
         }

ledger/src/tcpsigner/Makefile

@@ -25,6 +25,7 @@ COMMONPATH = ../common/src
 VPATH = $(SRCPATH):$(COMMONPATH)
 
 CFLAGS += -g -O0 -I$(SRCPATH) -I$(COMMONPATH) -I. -I /usr/local/include -DHSM_SIMULATOR
+CFLAGS += -Werror
 
 LDFLAGS = -lsecp256k1
 

ledger/src/ui/Makefile

@@ -133,6 +133,7 @@ CFLAGS   += -std=gnu99 -Werror=int-to-pointer-cast -Wall -Wextra #-save-temps
 CFLAGS   += -fdata-sections -ffunction-sections -funsigned-char -fshort-enums
 CFLAGS   += -mno-unaligned-access
 CFLAGS   += -Wno-unused-parameter -Wno-duplicate-decl-specifier
+CFLAGS   += -Werror
 ifeq ($(DEBUG_BUILD),1)
 CFLAGS   += -DDEBUG_BUILD=${DEBUG_BUILD}
 endif

ledger/src/ui/src/attestation.c

@@ -151,7 +151,7 @@ unsigned int get_attestation(volatile unsigned int rx, att_t* att_ctx) {
         SAFE_MEMMOVE(att_ctx->msg,
                      sizeof(att_ctx->msg),
                      att_ctx->msg_offset,
-                     PIC(att_msg_prefix),
+                     (const void*)PIC(att_msg_prefix),
                      sizeof(att_msg_prefix),
                      MEMMOVE_ZERO_OFFSET,
                      sizeof(att_msg_prefix),
@@ -173,14 +173,14 @@ unsigned int get_attestation(volatile unsigned int rx, att_t* att_ctx) {
                 SAFE_MEMMOVE(att_ctx->path,
                              sizeof(att_ctx->path),
                              MEMMOVE_ZERO_OFFSET,
-                             PIC(key_derivation_path),
+                             (const void*)PIC(key_derivation_path),
                              sizeof(key_derivation_path),
                              MEMMOVE_ZERO_OFFSET,
                              sizeof(key_derivation_path),
                              THROW(INTERNAL));
                 // Derive private key
                 os_perso_derive_node_bip32(CX_CURVE_256K1,
-                                           att_ctx->path,
+                                           (unsigned int*)att_ctx->path,
                                            PATH_PART_COUNT,
                                            att_ctx->priv_key_data,
                                            NULL);

ledger/src/ui/src/bolos_ux.c

@@ -61,7 +61,7 @@ unsigned char G_pin_buffer[PIN_LENGTH + 2];
 // Skip the prepended length of pin buffer
 #define G_PIN_BUFFER_PAYLOAD (G_pin_buffer + 1)
 // Unify pin buffer length
-#define G_PIN_BUFFER_LEN() strlen(G_PIN_BUFFER_PAYLOAD)
+#define G_PIN_BUFFER_LEN() strlen((const char *)G_PIN_BUFFER_PAYLOAD)
 
 #ifdef OS_IO_SEPROXYHAL
 
@@ -125,11 +125,11 @@ unsigned short io_timeout(unsigned short last_timeout) {
 void screen_hex_identifier_string_buffer(const unsigned char *buffer,
                                          unsigned int total) {
     SPRINTF(G_bolos_ux_context.string_buffer,
-            "%.*H...%.*H",
+            "%.*hhX...%.*hhX",
             BOLOS_UX_HASH_LENGTH / 2,
-            buffer,
+            (unsigned char)buffer,
             BOLOS_UX_HASH_LENGTH / 2,
-            buffer + total - BOLOS_UX_HASH_LENGTH / 2);
+            (unsigned char)(buffer + total - BOLOS_UX_HASH_LENGTH / 2));
 }
 
 // common code for all screens
@@ -500,8 +500,8 @@ static void sample_main(void) {
                 case RSK_SEED_CMD: // Send wordlist
                     reset_if_starting(RSK_META_CMD_UIOP);
                     pin = APDU_AT(2);
-                    if ((pin >= 0) &&
-                        (pin <= sizeof(G_bolos_ux_context.words_buffer)))
+                    if ((pin >= 0) && ((unsigned int)(pin) <=
+                                       sizeof(G_bolos_ux_context.words_buffer)))
                         G_bolos_ux_context.words_buffer[pin] = APDU_AT(3);
                     THROW(APDU_OK);
                     break;
@@ -530,7 +530,8 @@ static void sample_main(void) {
                     // Reset the onboarding flag to mark onboarding
                     // hasn't been done just in case something fails
                     aux = 0;
-                    nvm_write((void *)PIC(N_onboarded_ui), &aux, sizeof(aux));
+                    nvm_write(
+                        (void *)PIC(N_onboarded_ui), (void *)&aux, sizeof(aux));
 
 #ifndef DEBUG_BUILD
                     if (!is_pin_valid(G_PIN_BUFFER_PAYLOAD)) {
@@ -590,7 +591,8 @@ static void sample_main(void) {
                     // Turn the onboarding flag on to mark onboarding
                     // has been done using the UI
                     aux = 1;
-                    nvm_write((void *)PIC(N_onboarded_ui), &aux, sizeof(aux));
+                    nvm_write(
+                        (void *)PIC(N_onboarded_ui), (void *)&aux, sizeof(aux));
                     // Output
                     tx = 3;
                     THROW(APDU_OK);
@@ -649,9 +651,10 @@ static void sample_main(void) {
                     // RSK_UNLOCK_CMD does not send the prepended length,
                     // so we kept this call backwards compatible, using
                     // G_pin_buffer instead of G_PIN_BUFFER_PAYLOAD
-                    SET_APDU_AT(2,
-                                os_global_pin_check(G_pin_buffer,
-                                                    strlen(G_pin_buffer)));
+                    SET_APDU_AT(
+                        2,
+                        os_global_pin_check(
+                            G_pin_buffer, strlen((const char *)G_pin_buffer)));
                     tx = 5;
                     THROW(APDU_OK);
                     // The pin value will also be used in
@@ -952,7 +955,8 @@ void bolos_ux_main(void) {
                 // PIN is invalidated so we must check it again. The pin value
                 // used here is the same as in RSK_UNLOCK_CMD, so we also
                 // don't have a prepended length byte
-                os_global_pin_check(G_pin_buffer, strlen(G_pin_buffer));
+                os_global_pin_check(G_pin_buffer,
+                                    strlen((const char *)G_pin_buffer));
                 G_bolos_ux_context.exit_code = BOLOS_UX_OK;
                 explicit_bzero(G_pin_buffer, sizeof(G_pin_buffer));
                 break;

ledger/src/ui/src/bolos_ux.h

@@ -297,6 +297,9 @@ void bolos_ux_main(void);
 // Check if app is allowed, and manage app blacklist
 int is_app_version_allowed(application_t *app);
 
+void screen_display_init(unsigned int stack_slot);
+void screen_state_init(unsigned int stack_slot);
+
 #endif // HAVE_BOLOS_UX
 
 #endif // BOLOS_UX_H

ledger/src/ui/src/bolos_ux_common_keyboard.c

@@ -383,7 +383,7 @@ const bagl_element_t *screen_common_keyboard_before_element_display_callback(
     const bagl_element_t *element) {
     // copy element to be displayed
     os_memmove(&G_bolos_ux_context.tmp_element,
-               PIC(element),
+               (void *)PIC(element),
                sizeof(G_bolos_ux_context.tmp_element));
 
     switch (element->component.userid) {

ledger/src/ui/src/bolos_ux_common_pin.c

@@ -354,18 +354,18 @@ const bagl_element_t *screen_common_pin_before_element_display_callback(
             if (digit_count > (element->component.userid & 0xF)) {
                 // display the * (validated digits)
                 os_memmove(G_bolos_ux_context.string_buffer + 4,
-                           PIC(C_digit_font[0xA]),
+                           (void *)PIC(C_digit_font[0xA]),
                            sizeof(bagl_icon_details_t));
             } else if (digit_count < (element->component.userid & 0xF)) {
                 // display the _ (not entered digits)
                 os_memmove(G_bolos_ux_context.string_buffer + 4,
-                           PIC(C_digit_font[0xB]),
+                           (void *)PIC(C_digit_font[0xB]),
                            sizeof(bagl_icon_details_t));
             } else if (digit_count == (element->component.userid & 0xF)) {
                 if (G_bolos_ux_context.string_buffer[0] == ':') {
                     // display the 'backspace'
                     os_memmove(G_bolos_ux_context.string_buffer + 4,
-                               PIC(C_digit_font[0xC]),
+                               (void *)PIC(C_digit_font[0xC]),
                                sizeof(bagl_icon_details_t));
                     os_memmove(&G_bolos_ux_context.tmp_element,
                                element,
@@ -376,7 +376,7 @@ const bagl_element_t *screen_common_pin_before_element_display_callback(
                 } else if (G_bolos_ux_context.string_buffer[0] == ';') {
                     // display the 'check'
                     os_memmove(G_bolos_ux_context.string_buffer + 4,
-                               PIC(C_digit_font[0xD]),
+                               (void *)PIC(C_digit_font[0xD]),
                                sizeof(bagl_icon_details_t));
                     // change the shape for the check
                     os_memmove(&G_bolos_ux_context.tmp_element,
@@ -388,7 +388,8 @@ const bagl_element_t *screen_common_pin_before_element_display_callback(
                     // display the digit
                     os_memmove(
                         G_bolos_ux_context.string_buffer + 4,
-                        PIC(C_digit_font[G_bolos_ux_context.string_buffer[0] -
+                        (void *)PIC(
+                            C_digit_font[G_bolos_ux_context.string_buffer[0] -
                                          '0']),
                         sizeof(bagl_icon_details_t));
                 }

ledger/src/ui/src/bolos_ux_dashboard.c

@@ -512,8 +512,12 @@ void screen_dashboard_disable_bolos_before_app(void) {
 unsigned int screen_dashboard_unsigned_app_button(
     unsigned int button_mask, unsigned int button_mask_counter) {
     UNUSED(button_mask_counter);
+
+#ifdef DEBUG_BUILD
     screen_dashboard_t db;
     int i = 0;
+#endif
+
     switch (button_mask) {
     case BUTTON_EVT_RELEASED | BUTTON_LEFT:
         screen_dashboard_init();

ledger/src/ui/src/pin.c

@@ -42,7 +42,7 @@
  */
 bool is_pin_valid(unsigned char *pin) {
     // PIN_LENGTH is the only length accepted
-    size_t length = strnlen(pin, PIN_LENGTH + 1);
+    size_t length = strnlen((const char *)pin, PIN_LENGTH + 1);
     if (length != PIN_LENGTH) {
         return false;
     }