From 2b0e4e44b94a4edcdf838912731e4c971e8f34c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Desbiens?= Date: Sat, 8 Aug 2026 08:09:06 -0400 Subject: [PATCH] Hardened the module converter utilities against malformed input (#580) * Hardened the module converter utilities against malformed input While reviewing the code_buffer leak reported in issue 571, three further pre-existing defects turned up in the same host-side utilities. The four ELF area allocations in module_to_binary.c and module_to_c_array.c were unchecked, and every elf_object_read() return value was discarded, so a truncated or crafted ELF file was read into whatever the allocation and the reads happened to leave behind. Check each allocation, distinguishing a NULL return for an empty area from a genuine failure, and abandon the conversion with exit code 5 on an allocation failure and exit code 6 on a read failure. Validate the section string table index taken from the ELF header before it is used to subscript the section header area. AddressSanitizer confirms that an out-of-range index produced a heap buffer overflow in both tools. Correct the address format specifiers in module_to_c_array.c and module_binary_to_c_array.c, which passed an unsigned long to %08X, and close the source file on the invalid format path of module_binary_to_c_array.c. The unused current_total local is removed. All three utilities now build warning free with gcc -std=c99 -Wall -Wextra, and the code they emit is unchanged byte for byte on valid input. Refresh the version banners of all three tools, on the console and in the header written into the generated C arrays, to the 2024 Microsoft Corp and 2026 Eclipse ThreadX contributors copyrights and version v6.5.2.202603. The banners still advertised v5.8 and v5.4 with a 2018 build date. The .exe suffix is dropped from the tool names, since these tools build on Linux too. Related to https://github.com/eclipse-threadx/threadx/issues/571 Assisted-by: Claude Code (Opus 5) * Added the missing licence header to module_binary_to_c_array.c The file carried no copyright or licence header at all, unlike the two other converter utilities in the same directory. Use the same MIT header they carry, since the three tools share an origin. Assisted-by: Claude Code (Opus 5) --- .../utilities/module_binary_to_c_array.c | 30 ++- .../utilities/module_to_binary.c | 209 +++++++++++++++-- .../utilities/module_to_c_array.c | 217 ++++++++++++++++-- 3 files changed, 407 insertions(+), 49 deletions(-) diff --git a/common_modules/module_manager/utilities/module_binary_to_c_array.c b/common_modules/module_manager/utilities/module_binary_to_c_array.c index da0cf97c..38b38c32 100644 --- a/common_modules/module_manager/utilities/module_binary_to_c_array.c +++ b/common_modules/module_manager/utilities/module_binary_to_c_array.c @@ -1,3 +1,16 @@ +/***************************************************************************/ +/* Copyright (c) 2024 Microsoft Corporation */ +/* Copyright (c) 2026 Eclipse ThreadX contributors */ +/* */ +/* This program and the accompanying materials are made available under */ +/* the terms of the MIT License which is available at */ +/* https://opensource.org/licenses/MIT. */ +/* */ +/* SPDX-License-Identifier: MIT */ +/***************************************************************************/ + +// Some portions generated by Claude Code (Opus 5) + #include #include #include @@ -25,8 +38,11 @@ unsigned long column; { /* Print an error message out and wait for user key hit. */ - printf("module_binary_to_c_array.exe - Copyright (c) Microsoft Corporation v5.8\n"); - printf("**** Error: invalid input parameter for module_binary_to_c_array.exe **** \n"); + printf("module_binary_to_c_array\n"); + printf("(c) 2024 Microsoft Corp\n"); + printf("(c) 2026 Eclipse ThreadX contributors\n"); + printf("v6.5.2.202603\n"); + printf("**** Error: invalid input parameter for module_binary_to_c_array **** \n"); printf(" Command Line Should be:\n\n"); printf(" > module_binary_to_c_array source_binary_file c_array_file \n\n"); return(1); @@ -57,6 +73,10 @@ unsigned long column; /* Print an error message out and wait for user key hit. */ printf("**** Error: invalid format of binary input file **** \n"); printf(" File: %s ", argv[1]); + + /* Close the source file. */ + fclose(source_file); + return(3); } @@ -75,7 +95,9 @@ unsigned long column; fprintf(array_file, "/**************************** Module-Binary-to-C-array Utility **********************************/\n"); fprintf(array_file, "/* */\n"); - fprintf(array_file, "/* Copyright (c) Microsoft Corporation Version 5.4, build date: 03-01-2018 */\n"); + fprintf(array_file, "/* Copyright (c) 2024 Microsoft Corp */\n"); + fprintf(array_file, "/* Copyright (c) 2026 Eclipse ThreadX contributors */\n"); + fprintf(array_file, "/* v6.5.2.202603 */\n"); fprintf(array_file, "/* */\n"); fprintf(array_file, "/************************************************************************************************/\n\n"); fprintf(array_file, "/* \n"); @@ -109,7 +131,7 @@ unsigned long column; { if (address != 0) fprintf(array_file, ",\n"); - fprintf(array_file, "/* 0x%08X */ 0x%02X", address, (unsigned int) alpha); + fprintf(array_file, "/* 0x%08lX */ 0x%02X", address, (unsigned int) alpha); } else fprintf(array_file, ", 0x%02X", (unsigned int) alpha); diff --git a/common_modules/module_manager/utilities/module_to_binary.c b/common_modules/module_manager/utilities/module_to_binary.c index 66657fcc..b691d9e4 100644 --- a/common_modules/module_manager/utilities/module_to_binary.c +++ b/common_modules/module_manager/utilities/module_to_binary.c @@ -22,6 +22,11 @@ that are only known at run time. Every allocation result is checked for NULL and every buffer is released before its pointer is reused. */ +/* MISRA C:2012 Rule 15.5 (advisory) deviation: main() returns from several + error paths instead of having a single point of exit. The project forbids + goto, so an early return is the only way to abandon the conversion, and this + matches the style already used by the parameter and file open checks. */ + /* Define the file handles. */ @@ -146,13 +151,52 @@ unsigned char *buffer; } +/* Close the files and release the ELF areas allocated by main(). This is called + from every exit path taken after the files have been opened, so it must + tolerate being called with resources that were never acquired. The areas it + releases are the file scope pointers declared above. */ + +static void converter_cleanup(void) +{ + + /* Determine if the source file is open. */ + if (source_file != NULL) + { + + /* Close it. */ + fclose(source_file); + source_file = NULL; + } + + /* Determine if the binary output file is open. */ + if (binary_file != NULL) + { + + /* Close it. */ + fclose(binary_file); + binary_file = NULL; + } + + /* Release the ELF areas. Note that free() is defined to do nothing when it + is supplied a NULL pointer, so no test is required here. */ + free(program_header); + program_header = NULL; + free(section_header); + section_header = NULL; + free(section_string_table); + section_string_table = NULL; + free(code_section_array); + code_section_array = NULL; +} + + int main(int argc, char* argv[]) { unsigned long i, j; -unsigned long current_total; unsigned long address; unsigned long size; +unsigned long allocation_size; unsigned char *code_buffer; unsigned long code_section_index; CODE_SECTION_ENTRY code_section_temp; @@ -164,8 +208,11 @@ unsigned char zero_value; { /* Print an error message out and wait for user key hit. */ - printf("module_to_binary.exe - Copyright (c) Microsoft Corporation v5.8\n"); - printf("**** Error: invalid input parameter for module_to_binary.exe **** \n"); + printf("module_to_binary\n"); + printf("(c) 2024 Microsoft Corp\n"); + printf("(c) 2026 Eclipse ThreadX contributors\n"); + printf("v6.5.2.202603\n"); + printf("**** Error: invalid input parameter for module_to_binary **** \n"); printf(" Command Line Should be:\n\n"); printf(" > module_to_binary source_elf_file c_binary_file \n\n"); return(1); @@ -198,29 +245,140 @@ unsigned char zero_value; } /* Read the ELF header. */ - elf_object_read(0, &header, sizeof(header)); + if (elf_object_read(0, &header, sizeof(header)) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on the ELF header **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } /* Allocate memory for the program header(s). */ - program_header = malloc(sizeof(ELF_PROGRAM_HEADER)*header.elf_header_program_header_entries); + allocation_size = sizeof(ELF_PROGRAM_HEADER)*header.elf_header_program_header_entries; + program_header = malloc(allocation_size); + + /* Determine if the memory allocation was successful. Note that an empty area + is not an error, since malloc() is permitted to return NULL for it. */ + if ((program_header == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the program header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } /* Read the program header(s). */ - elf_object_read(header.elf_header_program_header_offset, program_header, (sizeof(ELF_PROGRAM_HEADER)*header.elf_header_program_header_entries)); + if (elf_object_read(header.elf_header_program_header_offset, program_header, allocation_size) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on the program header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } /* Allocate memory for the section header(s). */ - section_header = malloc(sizeof(ELF_SECTION_HEADER)*header.elf_header_section_header_entries); + allocation_size = sizeof(ELF_SECTION_HEADER)*header.elf_header_section_header_entries; + section_header = malloc(allocation_size); + + /* Determine if the memory allocation was successful. */ + if ((section_header == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the section header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } /* Read the section header(s). */ - elf_object_read(header.elf_header_section_header_offset, section_header, (sizeof(ELF_SECTION_HEADER)*header.elf_header_section_header_entries)); + if (elf_object_read(header.elf_header_section_header_offset, section_header, allocation_size) != 0) + { + /* Print an error message out. */ + printf("**** Error: read failed on the section header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } + + /* Determine if the section string table index supplied by the ELF header is + inside the section header area that was just read. */ + if (header.elf_header_section_string_index >= header.elf_header_section_header_entries) + { + + /* Print an error message out. */ + printf("**** Error: invalid section string table index in the ELF header **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } /* Alocate memory for the section string table. */ - section_string_table = malloc(section_header[header.elf_header_section_string_index].elf_section_header_size); + allocation_size = section_header[header.elf_header_section_string_index].elf_section_header_size; + section_string_table = malloc(allocation_size); + + /* Determine if the memory allocation was successful. */ + if ((section_string_table == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the section string table **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } /* Read the section string table. */ - elf_object_read(section_header[header.elf_header_section_string_index].elf_section_header_offset, section_string_table, section_header[header.elf_header_section_string_index].elf_section_header_size); + if (elf_object_read(section_header[header.elf_header_section_string_index].elf_section_header_offset, section_string_table, allocation_size) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on the section string table **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } /* Allocate memory for the code section array. */ - code_section_array = malloc(sizeof(CODE_SECTION_ENTRY)*header.elf_header_section_header_entries); + allocation_size = sizeof(CODE_SECTION_ENTRY)*header.elf_header_section_header_entries; + code_section_array = malloc(allocation_size); + + /* Determine if the memory allocation was successful. */ + if ((code_section_array == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the code section array **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } + code_section_index = 0; /* Print out the section header(s). */ @@ -267,9 +425,8 @@ unsigned char zero_value; if (code_section_index == 0) { - /* Close files. */ - fclose(source_file); - fclose(binary_file); + /* Close the files and release the ELF areas. */ + converter_cleanup(); return(4); } @@ -326,16 +483,27 @@ unsigned char zero_value; /* Print an error message out. */ printf("**** Error: memory allocation failed for code section **** \n"); - /* Close files. */ - fclose(source_file); - fclose(binary_file); + /* Close the files and release the ELF areas. */ + converter_cleanup(); return(5); } /* Read in the code area. */ j = code_section_array[i].code_section_index; - elf_object_read(section_header[j].elf_section_header_offset, code_buffer, code_section_array[i].code_section_size); + if (elf_object_read(section_header[j].elf_section_header_offset, code_buffer, code_section_array[i].code_section_size) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on a code section **** \n"); + + /* Release the code section buffer, then close the files and release + the ELF areas. */ + free(code_buffer); + converter_cleanup(); + + return(6); + } /* Write out the contents of this program area. */ size = code_section_array[i].code_section_size; @@ -362,9 +530,8 @@ unsigned char zero_value; code_buffer = NULL; } - /* Close files. */ - fclose(source_file); - fclose(binary_file); + /* Close the files and release the ELF areas. */ + converter_cleanup(); return 0; } diff --git a/common_modules/module_manager/utilities/module_to_c_array.c b/common_modules/module_manager/utilities/module_to_c_array.c index 72461e27..554d13ba 100644 --- a/common_modules/module_manager/utilities/module_to_c_array.c +++ b/common_modules/module_manager/utilities/module_to_c_array.c @@ -22,6 +22,11 @@ that are only known at run time. Every allocation result is checked for NULL and every buffer is released before its pointer is reused. */ +/* MISRA C:2012 Rule 15.5 (advisory) deviation: main() returns from several + error paths instead of having a single point of exit. The project forbids + goto, so an early return is the only way to abandon the conversion, and this + matches the style already used by the parameter and file open checks. */ + /* Define the file handles. */ @@ -146,13 +151,52 @@ unsigned char *buffer; } +/* Close the files and release the ELF areas allocated by main(). This is called + from every exit path taken after the files have been opened, so it must + tolerate being called with resources that were never acquired. The areas it + releases are the file scope pointers declared above. */ + +static void converter_cleanup(void) +{ + + /* Determine if the source file is open. */ + if (source_file != NULL) + { + + /* Close it. */ + fclose(source_file); + source_file = NULL; + } + + /* Determine if the C array output file is open. */ + if (array_file != NULL) + { + + /* Close it. */ + fclose(array_file); + array_file = NULL; + } + + /* Release the ELF areas. Note that free() is defined to do nothing when it + is supplied a NULL pointer, so no test is required here. */ + free(program_header); + program_header = NULL; + free(section_header); + section_header = NULL; + free(section_string_table); + section_string_table = NULL; + free(code_section_array); + code_section_array = NULL; +} + + int main(int argc, char* argv[]) { unsigned long i, j, k; -unsigned long current_total; unsigned long address; unsigned long size; +unsigned long allocation_size; unsigned long column; unsigned char *code_buffer; unsigned long code_section_index; @@ -164,8 +208,11 @@ CODE_SECTION_ENTRY code_section_temp; { /* Print an error message out and wait for user key hit. */ - printf("module_to_c_array.exe - Copyright (c) Microsoft Corporation v5.8\n"); - printf("**** Error: invalid input parameter for module_to_c_array.exe **** \n"); + printf("module_to_c_array\n"); + printf("(c) 2024 Microsoft Corp\n"); + printf("(c) 2026 Eclipse ThreadX contributors\n"); + printf("v6.5.2.202603\n"); + printf("**** Error: invalid input parameter for module_to_c_array **** \n"); printf(" Command Line Should be:\n\n"); printf(" > module_to_c_array source_elf_file c_array_file \n\n"); return(1); @@ -198,11 +245,23 @@ CODE_SECTION_ENTRY code_section_temp; } /* Read the ELF header. */ - elf_object_read(0, &header, sizeof(header)); + if (elf_object_read(0, &header, sizeof(header)) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on the ELF header **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } fprintf(array_file, "/**************************** Module-to-C-array Utility *****************************************/\n"); fprintf(array_file, "/* */\n"); - fprintf(array_file, "/* Copyright (c) Microsoft Corporation Version 5.8, build date: 03-01-2018 */\n"); + fprintf(array_file, "/* Copyright (c) 2024 Microsoft Corp */\n"); + fprintf(array_file, "/* Copyright (c) 2026 Eclipse ThreadX contributors */\n"); + fprintf(array_file, "/* v6.5.2.202603 */\n"); fprintf(array_file, "/* */\n"); fprintf(array_file, "/************************************************************************************************/\n\n"); fprintf(array_file, "/* \n"); @@ -211,26 +270,127 @@ CODE_SECTION_ENTRY code_section_temp; fprintf(array_file, "*/\n\n"); /* Allocate memory for the program header(s). */ - program_header = malloc(sizeof(ELF_PROGRAM_HEADER)*header.elf_header_program_header_entries); + allocation_size = sizeof(ELF_PROGRAM_HEADER)*header.elf_header_program_header_entries; + program_header = malloc(allocation_size); + + /* Determine if the memory allocation was successful. Note that an empty area + is not an error, since malloc() is permitted to return NULL for it. */ + if ((program_header == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the program header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } /* Read the program header(s). */ - elf_object_read(header.elf_header_program_header_offset, program_header, (sizeof(ELF_PROGRAM_HEADER)*header.elf_header_program_header_entries)); + if (elf_object_read(header.elf_header_program_header_offset, program_header, allocation_size) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on the program header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } /* Allocate memory for the section header(s). */ - section_header = malloc(sizeof(ELF_SECTION_HEADER)*header.elf_header_section_header_entries); + allocation_size = sizeof(ELF_SECTION_HEADER)*header.elf_header_section_header_entries; + section_header = malloc(allocation_size); + + /* Determine if the memory allocation was successful. */ + if ((section_header == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the section header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } /* Read the section header(s). */ - elf_object_read(header.elf_header_section_header_offset, section_header, (sizeof(ELF_SECTION_HEADER)*header.elf_header_section_header_entries)); + if (elf_object_read(header.elf_header_section_header_offset, section_header, allocation_size) != 0) + { + /* Print an error message out. */ + printf("**** Error: read failed on the section header(s) **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } + + /* Determine if the section string table index supplied by the ELF header is + inside the section header area that was just read. */ + if (header.elf_header_section_string_index >= header.elf_header_section_header_entries) + { + + /* Print an error message out. */ + printf("**** Error: invalid section string table index in the ELF header **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } /* Alocate memory for the section string table. */ - section_string_table = malloc(section_header[header.elf_header_section_string_index].elf_section_header_size); + allocation_size = section_header[header.elf_header_section_string_index].elf_section_header_size; + section_string_table = malloc(allocation_size); + + /* Determine if the memory allocation was successful. */ + if ((section_string_table == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the section string table **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } /* Read the section string table. */ - elf_object_read(section_header[header.elf_header_section_string_index].elf_section_header_offset, section_string_table, section_header[header.elf_header_section_string_index].elf_section_header_size); + if (elf_object_read(section_header[header.elf_header_section_string_index].elf_section_header_offset, section_string_table, allocation_size) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on the section string table **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(6); + } /* Allocate memory for the code section array. */ - code_section_array = malloc(sizeof(CODE_SECTION_ENTRY)*header.elf_header_section_header_entries); + allocation_size = sizeof(CODE_SECTION_ENTRY)*header.elf_header_section_header_entries; + code_section_array = malloc(allocation_size); + + /* Determine if the memory allocation was successful. */ + if ((code_section_array == NULL) && (allocation_size != 0)) + { + + /* Print an error message out. */ + printf("**** Error: memory allocation failed for the code section array **** \n"); + + /* Close the files and release the ELF areas. */ + converter_cleanup(); + + return(5); + } + code_section_index = 0; /* Print out the section header(s). */ @@ -281,9 +441,8 @@ CODE_SECTION_ENTRY code_section_temp; fprintf(array_file, "unsigned char module_code[] = {0x00};\n\n"); - /* Close files. */ - fclose(source_file); - fclose(array_file); + /* Close the files and release the ELF areas. */ + converter_cleanup(); return(4); } @@ -330,7 +489,7 @@ CODE_SECTION_ENTRY code_section_temp; /* Print out a character with a leading comma, except on the first character. */ if (column == 0) - fprintf(array_file, "/* 0x%08X */ 0x00", address); + fprintf(array_file, "/* 0x%08lX */ 0x00", address); else fprintf(array_file, ", 0x00"); @@ -358,16 +517,27 @@ CODE_SECTION_ENTRY code_section_temp; /* Print an error message out. */ printf("**** Error: memory allocation failed for code section **** \n"); - /* Close files. */ - fclose(source_file); - fclose(array_file); + /* Close the files and release the ELF areas. */ + converter_cleanup(); return(5); } /* Read in the code area. */ j = code_section_array[i].code_section_index; - elf_object_read(section_header[j].elf_section_header_offset, code_buffer, code_section_array[i].code_section_size); + if (elf_object_read(section_header[j].elf_section_header_offset, code_buffer, code_section_array[i].code_section_size) != 0) + { + + /* Print an error message out. */ + printf("**** Error: read failed on a code section **** \n"); + + /* Release the code section buffer, then close the files and release + the ELF areas. */ + free(code_buffer); + converter_cleanup(); + + return(6); + } /* Write out the contents of this program area. */ size = code_section_array[i].code_section_size; @@ -379,7 +549,7 @@ CODE_SECTION_ENTRY code_section_temp; /* Print out a character with a leading comma, except on the first character. */ if (column == 0) - fprintf(array_file, "/* 0x%08X */ 0x%02X", address, (unsigned int) code_buffer[j]); + fprintf(array_file, "/* 0x%08lX */ 0x%02X", address, (unsigned int) code_buffer[j]); else fprintf(array_file, ", 0x%02X", (unsigned int) code_buffer[j]); @@ -422,9 +592,8 @@ CODE_SECTION_ENTRY code_section_temp; /* Finally, finish the C array containing the module code. */ fprintf(array_file, "};\n\n"); - /* Close files. */ - fclose(source_file); - fclose(array_file); + /* Close the files and release the ELF areas. */ + converter_cleanup(); return 0; }