Merge pull request #181 from henrygab/CodeQL_Fixes_dev

Fix issues found by CodeQL

.github/workflows/build.yaml

@@ -54,8 +54,14 @@ jobs:
       run: cmake -B ${{github.workspace}}/build -DCMAKE_BUILD_TYPE=${{env.BUILD_TYPE}} -DBP_PICO_PLATFORM=${{matrix.board}} -DCMAKE_C_COMPILER=C:\ProgramData\chocolatey\bin\arm-none-eabi-gcc.exe -DCMAKE_CXX_COMPILER=C:\ProgramData\chocolatey\bin\arm-none-eabi-g++.exe -G "Unix Makefiles"
 
     - name: Build
+      if: ${{ matrix.os == 'macos-latest'}}
+      run: cmake --build ${{github.workspace}}/build --target ${{env.BUILD_TARGET}}
+
+    - name: Build
+      if: ${{ matrix.os != 'macos-latest'}}
       run: cmake --build ${{github.workspace}}/build --parallel --target ${{env.BUILD_TARGET}}
-
+
+
     - name: Save Artifacts
       uses: actions/upload-artifact@v4
       with:

src/binmode/binio.c

@@ -50,7 +50,7 @@
 uint8_t binmode_debug = 0;
 
 void script_print(const char* str) {
-    for (uint8_t i = 0; i < strlen(str); i++) {
+    for (size_t i = 0; i < strlen(str); i++) {
         bin_tx_fifo_put(str[i]);
     }
 }

src/binmode/binio_helpers.c

@@ -12,7 +12,7 @@
 #include "modes.h"
 
 void script_send(const char* c, uint32_t len) {
-    for (uint8_t i = 0; i < len; i++) {
+    for (uint32_t i = 0; i < len; i++) {
         bin_tx_fifo_put(c[i]);
     }
 }

src/binmode/legacy4third.c

@@ -677,7 +677,7 @@ void legacy4third_mode(void) {
 
         cdc_buff = (uint8_t*)mem_alloc(CDCBUFF_SIZE + TMPBUFF_SIZE, 0);
         if (binmode_debug) {
-            printf("\r\ncdc_buff: 0x%08X\r\n", cdc_buff);
+            printf("\r\ncdc_buff: %p\r\n", cdc_buff);
         }
         printf("\r\nDone! Just execute flashrom or avrdude using the binary com port\r\n");
         tmpbuf = cdc_buff + CDCBUFF_SIZE;

src/commands/global/cmd_convert.c

@@ -31,7 +31,7 @@ void cmd_convert_base(uint32_t value, uint32_t num_bits) {
     printf(" %s=", ui_term_color_reset());
     ui_format_print_number_3(value, num_bits, df_bin);
     if (value >= ' ' && value <= '~') {
-        printf("= '%c' ", value);
+        printf("= '%c' ", (char)value);
     }
 }
 

src/commands/global/disk.c

@@ -68,21 +68,21 @@ static uint32_t hex_dump(
         if (!bytes_read) {
             // Flush last line
             if (flag_ascii) {
-                uint8_t rem = buf_off % row_size;
+                uint16_t rem = buf_off % row_size;
                 if (rem) {
-                    for (uint8_t j = 0; j < row_size - rem; j++) {
+                    for (uint16_t j = 0; j < row_size - rem; j++) {
                         printf("   ");
                     }
                     printf(" |");
-                    for (uint8_t j = 0; j < rem; j++) {
+                    for (uint16_t j = 0; j < rem; j++) {
                         printf("%c", PRINTABLE(buf[line_start_off + j]));
                     }
                     printf("|");
                 }
             }
             break;
         }
-        for (uint16_t i = 0; i < bytes_read; i++) {
+        for (UINT i = 0; i < bytes_read; i++) {
             if (print_addr) {
                 print_addr = false;
                 printf("%04x  ", shown_off);
@@ -93,7 +93,7 @@ static uint32_t hex_dump(
             if (!(buf_off % row_size)) {
                 if (flag_ascii) {
                     printf(" |");
-                    for (uint8_t j = 0; j < row_size; j++) {
+                    for (uint16_t j = 0; j < row_size; j++) {
                         printf("%c", PRINTABLE(buf[line_start_off + j]));
                     }
                     printf("|");

src/mjson/mjson.c

@@ -820,8 +820,32 @@ int mjson_merge(const char *s, int n, const char *s2, int n2,
   int koff, klen, voff, vlen, t, t2, k, off = 0, len = 0, comma = 0;
   if (n < 2) return len;
   len += fn("{", 1, userdata);
+
+  // Determine maximum path length from either buffer.
+  // Required because `alloca()` allocates memory by
+  // expanding the current stack frame, but does not
+  // release memory until the function returns.
+  // Allocating once (and only once) thus helps avoid
+  // stack overflow.
+  size_t max_path_length = 1;
+  do {
+    off = 0;
+    while ((off = mjson_next(s, n, off, &koff, &klen, &voff, &vlen, &t)) != 0) {
+      size_t current_path_length = (size_t) klen + 1;
+      if (current_path_length > max_path_length) max_path_length = current_path_length;
+    }
+    off = 0;
+    while ((off = mjson_next(s2, n2, off, &koff, &klen, &voff, &vlen, &t)) != 0) {
+      size_t current_path_length = (size_t) klen + 1;
+      if (current_path_length > max_path_length) max_path_length = current_path_length;
+    }
+  } while (0);
+  char * path = (char *) alloca(max_path_length);
+
+  // first update `s` where the keys exist in both `s` and `s2`
+  off = 0;
   while ((off = mjson_next(s, n, off, &koff, &klen, &voff, &vlen, &t)) != 0) {
-    char *path = (char *) alloca((size_t) klen + 1);
+    memset(path, 0, max_path_length);
     const char *val;
     memcpy(path, "$.", 2);
     memcpy(path + 2, s + koff + 1, (size_t) (klen - 2));
@@ -842,10 +866,11 @@ int mjson_merge(const char *s, int n, const char *s2, int n2,
     }
     comma = 1;
   }
+
   // Add missing keys
   off = 0;
   while ((off = mjson_next(s2, n2, off, &koff, &klen, &voff, &vlen, &t)) != 0) {
-    char *path = (char *) alloca((size_t) klen + 1);
+    memset(path, 0, max_path_length);
     const char *val;
     if (t == MJSON_TOK_NULL) continue;
     memcpy(path, "$.", 2);

src/syntax.c

@@ -197,7 +197,7 @@ SYNTAX_STATUS syntax_compile(void) {
 
         if (syntax_io.out[syntax_io.out_cnt].command >= SYN_AUX_OUTPUT) {
             if (syntax_io.out[syntax_io.out_cnt].has_bits == false) {
-                printf("Error: missing IO number for command %c at position %d. Try %c.0\r\n", c, current_position);
+                printf("Error: missing IO number for command %c at position %d. Try %c.0\r\n", c, current_position, c);
                 return SSTATUS_ERROR;
             }
 
@@ -262,7 +262,7 @@ void syntax_run_write(struct _syntax_io* syntax_io, uint32_t current_position) {
         syntax_io->in[syntax_io->in_cnt].error = SERR_ERROR;
         return;
     }
-    for (uint16_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
+    for (uint32_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
         if (j > 0) {
             syntax_io->in_cnt++;
             syntax_io->in[syntax_io->in_cnt] = syntax_io->out[current_position];
@@ -280,7 +280,7 @@ void syntax_run_read(struct _syntax_io* syntax_io, uint32_t current_position) {
     #ifdef SYNTAX_DEBUG
         printf("[DEBUG] repeat %d, pos %d, cmd: %d\r\n", syntax_io->out[current_position].repeat, current_position, syntax_io->out[current_position].command);
     #endif
-    for (uint16_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
+    for (uint32_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
         if (j > 0) {
             syntax_io->in_cnt++;
             syntax_io->in[syntax_io->in_cnt] = syntax_io->out[current_position];
@@ -341,7 +341,7 @@ void syntax_run_adc(struct _syntax_io* syntax_io, uint32_t current_position) {
 }
 
 void syntax_run_tick_clock(struct _syntax_io* syntax_io, uint32_t current_position) {
-    for (uint16_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
+    for (uint32_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
         modes[system_config.mode].protocol_tick_clock(&syntax_io->in[syntax_io->in_cnt], NULL);
     }
 }
@@ -364,7 +364,7 @@ void syntax_run_set_dat_low(struct _syntax_io* syntax_io, uint32_t current_posit
 
 void syntax_run_read_dat(struct _syntax_io* syntax_io, uint32_t current_position) {
     //TODO: reality check out slots, actually repeat the read?
-    for (uint16_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
+    for (uint32_t j = 0; j < syntax_io->out[current_position].repeat; j++) {
         modes[system_config.mode].protocol_bitr(&syntax_io->in[syntax_io->in_cnt], NULL);
     }
 }

src/toolbars/logic_bar.c

@@ -162,7 +162,7 @@ void logic_bar_redraw(uint32_t start_pos, uint32_t total_samples) {
 // add blank space
 void frame_blank(uint16_t height) {
     // add space to draw the box
-    for (uint8_t i = 0; i < height; i++) {
+    for (uint16_t i = 0; i < height; i++) {
         printf("\r\n"); // make space!
     }
 }

src/ui/ui_display.c

@@ -13,7 +13,7 @@
 #include "ui/ui_cmdln.h"
 
 bool ui_display_list(const struct ui_prompt* menu) {
-    for (uint8_t i = 0; i < (*menu).menu_items_count; i++) {
+    for (uint i = 0; i < (*menu).menu_items_count; i++) {
         printf(" %d. %s%s%s\r\n", i + 1, ui_term_color_info(), displays[i].display_name, ui_term_color_reset());
     }
 }

src/ui/ui_format.c

@@ -289,6 +289,6 @@ void ui_format_print_number(uint32_t d) {
         printf(".%d", system_config.num_bits);
     }
     if (system_config.num_bits == 8 && d >= ' ' && d <= '~') { // ASCII
-        printf(" (\'%c\')", ((d >= 0x20) && (d < 0x7E) ? d : 0x20));
+        printf(" (\'%c\')", (char)d);
     }
 }

src/ui/ui_lcd.c

@@ -164,7 +164,8 @@ void lcd_write_string(
         // depending on how the font fits in the bitmap,
         // there may or may not be enough right hand padding between characters
         // this adds a configurable amount of space
-        for (uint8_t pad = 0; pad < (*font).lookup[adjusted_c].height * (*font).right_padding; pad++) {
+        uint16_t needed_padding = (*font).lookup[adjusted_c].height * (*font).right_padding;
+        for (uint16_t pad = 0; pad < needed_padding; pad++) {
             spi_write_blocking(BP_SPI_PORT, back_color, 2);
         }
         (c)++;

src/ui/ui_mode.c

@@ -14,7 +14,7 @@
 #include "binmode/fala.h"
 
 bool ui_mode_list(const struct ui_prompt* menu) {
-    for (uint8_t i = 0; i < (*menu).menu_items_count; i++) {
+    for (uint i = 0; i < (*menu).menu_items_count; i++) {
         printf(" %d. %s%s%s\r\n", i + 1, ui_term_color_info(), modes[i].protocol_name, ui_term_color_reset());
     }
 }
@@ -180,7 +180,7 @@ bool int_display_menu(const struct ui_prompt* menu) {
            ui_term_color_info(),
            ui_const_display_formats[system_config.display_format],
            ui_term_color_reset());
-    for (uint8_t i = 0; i < (*menu).menu_items_count; i++) {
+    for (uint i = 0; i < (*menu).menu_items_count; i++) {
         printf(" %d. %s%s%s\r\n", i + 1, ui_term_color_info(), ui_const_display_formats[i], ui_term_color_reset());
     }
 }

src/ui/ui_term.c

@@ -687,7 +687,7 @@ void ui_term_progress_bar_update(uint32_t current, uint32_t total, ui_term_progr
 
     system_config.terminal_ansi_statusbar_pause = true;
     if ((previous_pct) > 0) {
-        for (uint8_t i = 0; i < (previous_pct); i++) // advance this many positions
+        for (uint32_t i = 0; i < (previous_pct); i++) // advance this many positions
         {
             printf("%s-", ui_term_color_prompt());
         }

src/usb_rx.c

@@ -106,7 +106,7 @@ void tud_cdc_rx_cb(uint8_t itf) {
         uint32_t count = tud_cdc_n_read(0, buf, 64);
 
         // while bytes available shove them in the buffer
-        for (uint8_t i = 0; i < count; i++) {
+        for (uint32_t i = 0; i < count; i++) {
             queue2_add_blocking(&rx_fifo, &buf[i]); // BUGBUG -- blocking call from ISR!
         }
     }
@@ -115,7 +115,7 @@ void tud_cdc_rx_cb(uint8_t itf) {
         uint32_t count = tud_cdc_n_read(1, buf, 64);
 
         // while bytes available shove them in the buffer
-        for (uint8_t i = 0; i < count; i++) {
+        for (uint32_t i = 0; i < count; i++) {
             queue2_add_blocking(&bin_rx_fifo, &buf[i]); // BUGBUG -- blocking call from ISR!
         }
     }