From 2d64aa37e7a36e1275656964a2491103a3fc540d Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 11 Jan 2024 10:41:54 -0800 Subject: [PATCH 01/36] add macho parser --- .../inject_hash/macho_parser/macho_parser.c | 153 ++++++++++++++++++ .../inject_hash/macho_parser/macho_parser.h | 40 +++++ 2 files changed, 193 insertions(+) create mode 100644 util/fipstools/inject_hash/macho_parser/macho_parser.c create mode 100644 util/fipstools/inject_hash/macho_parser/macho_parser.h diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c new file mode 100644 index 0000000000..9e801b0002 --- /dev/null +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -0,0 +1,153 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + +#include +#include +#include +#include + +#define LOG_ERROR(...) do { \ + fprintf(stderr, "File: %s, Line: %d, ", __FILE__, __LINE__); \ + fprintf(stderr, __VA_ARGS__); \ + fprintf(stderr, "\n"); \ +} while(0) + +#include "macho_parser.h" + +// Documentation for the Mach-O structs can be found in macho-o/loader.h and mach-o/nlist.h +int read_macho_file(const char *filename, MachOFile *macho) { + FILE *file = fopen(filename, "rb"); + if (!file) { + LOG_ERROR("Error opening file %s", filename); + return 0; + } + + fread(&macho->machHeader, sizeof(MachOHeader), 1, file); + if(macho->machHeader.magic != MH_MAGIC_64) { + LOG_ERROR("File is not a 64-bit Mach-O file"); + return 0; + } + + macho->loadCommands = malloc(macho->machHeader.sizeofcmds); + fread(macho->loadCommands, macho->machHeader.sizeofcmds, 1, file); + + // We're only looking for __text, __const in the __TEXT segment, and the string & symbol tables + macho->numSections = 4; + macho->sections = malloc(macho->numSections * sizeof(SectionInfo)); + + // Iterate through load commands again to populate section information + uint32_t sectionIndex = 0; + for (uint32_t i = 0; i < macho->machHeader.sizeofcmds / BIT_MODIFIER; i += macho->loadCommands[i].cmdsize / BIT_MODIFIER) { + if (macho->loadCommands[i].cmd == LC_SEG) { + SegmentLoadCommand *segment = (SegmentLoadCommand *)&macho->loadCommands[i]; + if (strcmp(segment->segname, "__TEXT") == 0) { + SectionHeader *sections = (SectionHeader *)&segment[1]; + for (uint32_t j = 0; j < segment->nsects; j++) { + if (strcmp(sections[j].sectname, "__text") == 0 || strcmp(sections[j].sectname, "__const") == 0) { + macho->sections[sectionIndex].offset = sections[j].offset; + macho->sections[sectionIndex].size = sections[j].size; + macho->sections[sectionIndex].name = strdup(sections[j].sectname); + sectionIndex++; + } + } + } + + } else if (macho->loadCommands[i].cmd == LC_SYMTAB) { + SymtabLoadCommand *symtab = (SymtabLoadCommand *)&macho->loadCommands[i]; + macho->sections[sectionIndex].offset = symtab->symoff; + macho->sections[sectionIndex].size = symtab->nsyms * sizeof(nList); + macho->sections[sectionIndex].name = strdup("__symbol_table"); + sectionIndex++; + macho->sections[sectionIndex].offset = symtab->stroff; + macho->sections[sectionIndex].size = symtab->strsize; + macho->sections[sectionIndex].name = strdup("__string_table"); + sectionIndex++; + } + } + + fclose(file); + return 1; +} + +void free_macho_file(MachOFile *macho) { + free(macho->loadCommands); + for (uint32_t i = 0; i < macho->numSections; i++) { + free(macho->sections[i].name); + } + free(macho->sections); +} + +void print_macho_section_info(MachOFile *macho) { + printf("Number of sections: %u\n", macho->numSections); + for (uint32_t i = 0; i < macho->numSections; i++) { + printf("Section: %s, Offset: %u, Size: %zu\n", macho->sections[i].name, + macho->sections[i].offset, macho->sections[i].size); + } +} + +uint8_t* get_macho_section_data(char *filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset) { + FILE *file = fopen(filename, "rb"); + if (!file) { + LOG_ERROR("Error opening file %s", filename); + return NULL; + } + for (uint32_t i = 0; i < macho->numSections; i++) { + if (strcmp(macho->sections[i].name, sectionName) == 0) { + uint8_t *sectionData = malloc(macho->sections[i].size); + if (!sectionData) { + fclose(file); + LOG_ERROR("Error allocating memory for section data"); + return NULL; + } + + fseek(file, macho->sections[i].offset, SEEK_SET); + fread(sectionData, 1, macho->sections[i].size, file); + + if (size != NULL) { + *size = macho->sections[i].size; + } + if (offset) { + *offset = macho->sections[i].offset; + } + + fclose(file); + return sectionData; + } + } + + LOG_ERROR("Section %s not found in macho file %s", sectionName, filename); + fclose(file); + return NULL; +} + +uint32_t find_macho_symbol_index(uint8_t *symbolTableData, size_t symbolTableSize, uint8_t *stringTableData, size_t stringTableSize, const char *symbolName, uint32_t *base) { + if (symbolTableData == NULL || stringTableData == NULL) { + LOG_ERROR("Symbol and string table pointers cannot be null to find the symbol index"); + return 0; + } + + char* stringTable = malloc(stringTableSize); + memcpy(stringTable, stringTableData, stringTableSize); + + int found = 0; + uint32_t index = 0; + for (uint32_t i = 0; i < symbolTableSize / sizeof(nList); i++) { + nList *symbol = (nList *)(symbolTableData + i * sizeof(nList)); + if (strcmp(symbolName, &stringTable[symbol->n_un.n_strx]) == 0) { + if (!found) { + index = symbol->n_value; + found = 1; + } else { + LOG_ERROR("Duplicate symbol %s found", symbolName); + return 0; + } + + } + } + + free(stringTable); + if (base) { + index = index - *base; + } + return index; +} diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h new file mode 100644 index 0000000000..a714638740 --- /dev/null +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -0,0 +1,40 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + +#ifndef MACHO_PARSER_H +#define MACHO_PARSER_H + +#include +#include + +typedef struct { + uint32_t offset; + size_t size; + char *name; +} SectionInfo; + +// Since we only support 64-bit architectures on Apple, we don't need to account for any of the 32-bit structures +#define LC_SEG LC_SEGMENT_64 +#define BIT_MODIFIER 8 + +typedef struct mach_header_64 MachOHeader; +typedef struct load_command LoadCommand; +typedef struct segment_command_64 SegmentLoadCommand; +typedef struct symtab_command SymtabLoadCommand; +typedef struct section_64 SectionHeader; +typedef struct nlist_64 nList; + +typedef struct { + MachOHeader machHeader; + LoadCommand *loadCommands; + SectionInfo *sections; + uint32_t numSections; +} MachOFile; + +int read_macho_file(const char *filename, MachOFile *macho); +void free_macho_file(MachOFile *macho); +void print_macho_section_info(MachOFile *macho); +uint8_t* get_macho_section_data(char* filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset); +uint32_t find_macho_symbol_index(uint8_t *sectionData, size_t sectionSize, uint8_t *stringTableData, size_t stringTableSize, const char *symbolName, uint32_t *base); + +#endif From 0a5d6eebb1becbbff5cdde8d3c8f7e1914075be7 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 11 Jan 2024 10:42:06 -0800 Subject: [PATCH 02/36] add infra for macho parser tests --- util/fipstools/CMakeLists.txt | 2 + .../macho_parser/tests/CMakeLists.txt | 8 ++++ .../macho_parser/tests/macho_tests.c | 39 +++++++++++++++++++ 3 files changed, 49 insertions(+) create mode 100644 util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt create mode 100644 util/fipstools/inject_hash/macho_parser/tests/macho_tests.c diff --git a/util/fipstools/CMakeLists.txt b/util/fipstools/CMakeLists.txt index 79b7f044a7..6e24762871 100644 --- a/util/fipstools/CMakeLists.txt +++ b/util/fipstools/CMakeLists.txt @@ -6,4 +6,6 @@ if(FIPS AND BUILD_TESTING) ) target_link_libraries(test_fips crypto) target_include_directories(test_fips BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include) + + add_subdirectory(macho_parser/tests) endif() diff --git a/util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt b/util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt new file mode 100644 index 0000000000..2c4b270c8d --- /dev/null +++ b/util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt @@ -0,0 +1,8 @@ +if(FIPS AND APPLE) + add_executable( + test_macho_parser + + macho_tests.c + ../macho_parser.c + ) +endif() diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c new file mode 100644 index 0000000000..d478c3371a --- /dev/null +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -0,0 +1,39 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + +#include +#include + +#include "../macho_parser.h" + +static void test_read_macho_file(void) { + assert (1 == 1); +} + +static void test_free_macho_file(void) { + assert (1 == 1); +} + +static void test_print_macho_section_info(void) { + assert (1 == 1); +} + +static void test_get_macho_section_data(void) { + assert (1 == 1); +} + +static void test_find_macho_symbol_index(void) { + assert (1 == 1); +} + +int main(int argc, char *argv[]) { + + test_read_macho_file(); + test_free_macho_file(); + test_print_macho_section_info(); + test_get_macho_section_data(); + test_find_macho_symbol_index(); + + printf("All tests passed\n"); + return 0; +} From b9048b5ba24e9d9441eb0ad505d4d6c052574492 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 11 Jan 2024 15:03:50 -0800 Subject: [PATCH 03/36] add macho testing file --- util/fipstools/CMakeLists.txt | 2 +- .../inject_hash/macho_parser/common.h | 17 +++ .../inject_hash/macho_parser/macho_parser.c | 16 +-- .../macho_parser/tests/macho_tests.c | 130 +++++++++++++++++- 4 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 util/fipstools/inject_hash/macho_parser/common.h diff --git a/util/fipstools/CMakeLists.txt b/util/fipstools/CMakeLists.txt index 6e24762871..6a0d83efba 100644 --- a/util/fipstools/CMakeLists.txt +++ b/util/fipstools/CMakeLists.txt @@ -7,5 +7,5 @@ if(FIPS AND BUILD_TESTING) target_link_libraries(test_fips crypto) target_include_directories(test_fips BEFORE PRIVATE ${PROJECT_BINARY_DIR}/symbol_prefix_include) - add_subdirectory(macho_parser/tests) + add_subdirectory(inject_hash/macho_parser/tests) endif() diff --git a/util/fipstools/inject_hash/macho_parser/common.h b/util/fipstools/inject_hash/macho_parser/common.h new file mode 100644 index 0000000000..d1d81fbaaa --- /dev/null +++ b/util/fipstools/inject_hash/macho_parser/common.h @@ -0,0 +1,17 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + +#ifndef COMMON_H +#define COMMON_H + +#include +#include +#include + +#define LOG_ERROR(...) do { \ + fprintf(stderr, "File: %s, Line: %d, ", __FILE__, __LINE__); \ + fprintf(stderr, __VA_ARGS__); \ + fprintf(stderr, "\n"); \ +} while(0) + +#endif diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 9e801b0002..abc6ecd92b 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -2,18 +2,16 @@ // SPDX-License-Identifier: Apache-2.0 OR ISC #include -#include -#include -#include - -#define LOG_ERROR(...) do { \ - fprintf(stderr, "File: %s, Line: %d, ", __FILE__, __LINE__); \ - fprintf(stderr, __VA_ARGS__); \ - fprintf(stderr, "\n"); \ -} while(0) +#include "common.h" #include "macho_parser.h" +/** + * TODOs + * use goto for cleaner exits + * make all variable and function names snake case +*/ + // Documentation for the Mach-O structs can be found in macho-o/loader.h and mach-o/nlist.h int read_macho_file(const char *filename, MachOFile *macho) { FILE *file = fopen(filename, "rb"); diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index d478c3371a..a44f615ffc 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -2,12 +2,134 @@ // SPDX-License-Identifier: Apache-2.0 OR ISC #include -#include +#include "../common.h" #include "../macho_parser.h" +#define TEST_FILE "test_macho" + +static void create_test_macho_file(void) { + FILE *file = fopen(TEST_FILE, "wb"); + if (!file) { + LOG_ERROR("Error with fopen() on %s file", TEST_FILE); + exit(EXIT_FAILURE); + } + + /** + * macho file contents in order + * header - 0 + * text segment/command - sizeof(header) + * text section - sizeof(header) + sizeof(text segment) + * const section - sizeof(header) + sizeof(text segment) + sizeof(text section) + * symtab command - sizeof(header) + sizeof(text segment) + 2 * sizeof(sections) + * test section data - sizeof(all above) + * const section data - sizeof(all above except test section data) + text_section->size + * symbol table - sizeof(all above) + * string table - sizeof(all above) + */ + + MachOHeader test_header = { + .magic = MH_MAGIC_64, + .ncmds = 2, + .sizeofcmds = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader), + }; + + SegmentLoadCommand test_text_segment = { + .cmd = LC_SEGMENT_64, + .cmdsize = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader), + .segname = "__TEXT", + .nsects = 2, + }; + + SectionHeader test_text_section = { + .sectname = "__text", + .size = 1, // {0xC3} + .offset = sizeof(MachOHeader) + sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand), + }; + + SectionHeader test_const_section = { + .sectname = "__const", + .size = 2, // "hi" (including string literal) + .offset = test_text_section.offset + test_text_section.size, + }; + + SymtabLoadCommand test_symtab_command = { + .cmd = LC_SYMTAB, + .symoff = test_const_section.offset + test_const_section.size, + .nsyms = 2, + .stroff = test_const_section.offset + test_const_section.size + 2 * sizeof(nList), + .strsize = 2, + }; + + fwrite(&test_header, sizeof(MachOHeader), 1, file); + fwrite(&test_text_segment, sizeof(SegmentLoadCommand), 1, file); + fwrite(&test_text_section, sizeof(SectionHeader), 1, file); + fwrite(&test_const_section, sizeof(SectionHeader), 1, file); + fwrite(&test_symtab_command, sizeof(SymtabLoadCommand), 1, file); + + char test_text_data[] = {0xC3}; + char test_const_data[] = "hi"; + + fseek(file, test_text_section.offset, SEEK_SET); + fwrite(test_text_data, sizeof(test_text_data), 1, file); + + fseek(file, test_const_section.offset, SEEK_SET); + fwrite(test_const_data, sizeof(test_const_data), 1, file); + + // Leave out symbol and string tables for now + + // nList symbol1 = { + // .n_un = {.n_strx = 1}, // Index into the string table + // .n_type = N_TEXT, + // .n_sect = 1, + // .n_desc = 0, + // .n_value = 0x100000000, // Address of the symbol + // }; + + // nList symbol2 = { + // .n_un = {.n_strx = 15}, // Index into the string table + // .n_type = N_DATA, + // .n_sect = 2, + // .n_desc = 0, + // .n_value = 0x100000000 + sizeof(test_text_data), // Address of the symbol + // }; + + // fwrite(&symbol1, sizeof(struct nlist_64), 1, file); + // fwrite(&symbol2, sizeof(struct nlist_64), 1, file); + + // // Write the string table + // char string_table[] = "\0__text\0__const\0symbol1\0symbol2"; + // fseek(file, symtab_cmd.stroff, SEEK_SET); + // fwrite(string_table, sizeof(string_table), 1, file); + + if (fclose(file) != 0) { + LOG_ERROR("bad\n"); + } +} + +static void cleanup(void) { + if (remove(TEST_FILE) != 0) { + LOG_ERROR("Error deleting %s", TEST_FILE); + exit(EXIT_FAILURE); + } +} + static void test_read_macho_file(void) { - assert (1 == 1); + MachOFile test_macho_file; + if(!read_macho_file(TEST_FILE, &test_macho_file)) { + LOG_ERROR("Something in read_macho_file broke"); + exit(EXIT_FAILURE); + } + + if (test_macho_file.machHeader.magic != MH_MAGIC_64) { + LOG_ERROR("Incorrect header magic value"); + } + if (test_macho_file.machHeader.ncmds != 2) { + LOG_ERROR("Incorrect header ncmds value"); + } + if (test_macho_file.machHeader.sizeofcmds != sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader)) { + LOG_ERROR("Incorrect header sizeofcmds value"); + } } static void test_free_macho_file(void) { @@ -28,12 +150,16 @@ static void test_find_macho_symbol_index(void) { int main(int argc, char *argv[]) { + create_test_macho_file(); + test_read_macho_file(); test_free_macho_file(); test_print_macho_section_info(); test_get_macho_section_data(); test_find_macho_symbol_index(); + cleanup(); + printf("All tests passed\n"); return 0; } From 8ae5e57bb1c9ae6b98f7996069af04d46ed02682 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Fri, 12 Jan 2024 12:53:53 -0800 Subject: [PATCH 04/36] get tests working --- .../inject_hash/macho_parser/macho_parser.c | 18 ++++++++--------- .../inject_hash/macho_parser/macho_parser.h | 1 - .../macho_parser/tests/macho_tests.c | 20 ++++--------------- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index abc6ecd92b..62db4b46c3 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -26,8 +26,8 @@ int read_macho_file(const char *filename, MachOFile *macho) { return 0; } - macho->loadCommands = malloc(macho->machHeader.sizeofcmds); - fread(macho->loadCommands, macho->machHeader.sizeofcmds, 1, file); + LoadCommand *load_commands = malloc(macho->machHeader.sizeofcmds); + fread(load_commands, macho->machHeader.sizeofcmds, 1, file); // We're only looking for __text, __const in the __TEXT segment, and the string & symbol tables macho->numSections = 4; @@ -35,9 +35,9 @@ int read_macho_file(const char *filename, MachOFile *macho) { // Iterate through load commands again to populate section information uint32_t sectionIndex = 0; - for (uint32_t i = 0; i < macho->machHeader.sizeofcmds / BIT_MODIFIER; i += macho->loadCommands[i].cmdsize / BIT_MODIFIER) { - if (macho->loadCommands[i].cmd == LC_SEG) { - SegmentLoadCommand *segment = (SegmentLoadCommand *)&macho->loadCommands[i]; + for (uint32_t i = 0; i < macho->machHeader.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { + if (load_commands[i].cmd == LC_SEG) { + SegmentLoadCommand *segment = (SegmentLoadCommand *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { SectionHeader *sections = (SectionHeader *)&segment[1]; for (uint32_t j = 0; j < segment->nsects; j++) { @@ -49,9 +49,8 @@ int read_macho_file(const char *filename, MachOFile *macho) { } } } - - } else if (macho->loadCommands[i].cmd == LC_SYMTAB) { - SymtabLoadCommand *symtab = (SymtabLoadCommand *)&macho->loadCommands[i]; + } else if (load_commands[i].cmd == LC_SYMTAB) { + SymtabLoadCommand *symtab = (SymtabLoadCommand *)&load_commands[i]; macho->sections[sectionIndex].offset = symtab->symoff; macho->sections[sectionIndex].size = symtab->nsyms * sizeof(nList); macho->sections[sectionIndex].name = strdup("__symbol_table"); @@ -63,12 +62,13 @@ int read_macho_file(const char *filename, MachOFile *macho) { } } + + free(load_commands); fclose(file); return 1; } void free_macho_file(MachOFile *macho) { - free(macho->loadCommands); for (uint32_t i = 0; i < macho->numSections; i++) { free(macho->sections[i].name); } diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index a714638740..a5c20ccf1a 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -26,7 +26,6 @@ typedef struct nlist_64 nList; typedef struct { MachOHeader machHeader; - LoadCommand *loadCommands; SectionInfo *sections; uint32_t numSections; } MachOFile; diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index a44f615ffc..046d4388a9 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -15,23 +15,10 @@ static void create_test_macho_file(void) { exit(EXIT_FAILURE); } - /** - * macho file contents in order - * header - 0 - * text segment/command - sizeof(header) - * text section - sizeof(header) + sizeof(text segment) - * const section - sizeof(header) + sizeof(text segment) + sizeof(text section) - * symtab command - sizeof(header) + sizeof(text segment) + 2 * sizeof(sections) - * test section data - sizeof(all above) - * const section data - sizeof(all above except test section data) + text_section->size - * symbol table - sizeof(all above) - * string table - sizeof(all above) - */ - MachOHeader test_header = { .magic = MH_MAGIC_64, .ncmds = 2, - .sizeofcmds = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader), + .sizeofcmds = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand), }; SegmentLoadCommand test_text_segment = { @@ -55,6 +42,7 @@ static void create_test_macho_file(void) { SymtabLoadCommand test_symtab_command = { .cmd = LC_SYMTAB, + .cmdsize = sizeof(SymtabLoadCommand), .symoff = test_const_section.offset + test_const_section.size, .nsyms = 2, .stroff = test_const_section.offset + test_const_section.size + 2 * sizeof(nList), @@ -127,9 +115,10 @@ static void test_read_macho_file(void) { if (test_macho_file.machHeader.ncmds != 2) { LOG_ERROR("Incorrect header ncmds value"); } - if (test_macho_file.machHeader.sizeofcmds != sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader)) { + if (test_macho_file.machHeader.sizeofcmds != sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand)) { LOG_ERROR("Incorrect header sizeofcmds value"); } + } static void test_free_macho_file(void) { @@ -151,7 +140,6 @@ static void test_find_macho_symbol_index(void) { int main(int argc, char *argv[]) { create_test_macho_file(); - test_read_macho_file(); test_free_macho_file(); test_print_macho_section_info(); From 12a1afe2b07516590e8d8e7ec9d1bfc0cb61ad57 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Fri, 12 Jan 2024 14:06:38 -0800 Subject: [PATCH 05/36] more work on tests --- .../inject_hash/macho_parser/macho_parser.c | 6 +- .../inject_hash/macho_parser/macho_parser.h | 4 +- .../macho_parser/tests/macho_tests.c | 80 ++++++++++++++++--- 3 files changed, 73 insertions(+), 17 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 62db4b46c3..d7b6833a61 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -44,7 +44,7 @@ int read_macho_file(const char *filename, MachOFile *macho) { if (strcmp(sections[j].sectname, "__text") == 0 || strcmp(sections[j].sectname, "__const") == 0) { macho->sections[sectionIndex].offset = sections[j].offset; macho->sections[sectionIndex].size = sections[j].size; - macho->sections[sectionIndex].name = strdup(sections[j].sectname); + strcpy(macho->sections[sectionIndex].name, sections[j].sectname); sectionIndex++; } } @@ -53,11 +53,11 @@ int read_macho_file(const char *filename, MachOFile *macho) { SymtabLoadCommand *symtab = (SymtabLoadCommand *)&load_commands[i]; macho->sections[sectionIndex].offset = symtab->symoff; macho->sections[sectionIndex].size = symtab->nsyms * sizeof(nList); - macho->sections[sectionIndex].name = strdup("__symbol_table"); + strcpy(macho->sections[sectionIndex].name, "__symbol_table"); sectionIndex++; macho->sections[sectionIndex].offset = symtab->stroff; macho->sections[sectionIndex].size = symtab->strsize; - macho->sections[sectionIndex].name = strdup("__string_table"); + strcpy(macho->sections[sectionIndex].name, "__string_table"); sectionIndex++; } } diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index a5c20ccf1a..74a97ac059 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -8,9 +8,9 @@ #include typedef struct { - uint32_t offset; + char name[16]; size_t size; - char *name; + uint32_t offset; } SectionInfo; // Since we only support 64-bit architectures on Apple, we don't need to account for any of the 32-bit structures diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 046d4388a9..adcc72d7d6 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -8,7 +8,21 @@ #define TEST_FILE "test_macho" -static void create_test_macho_file(void) { +static void print_hex(const void *ptr, size_t size) { + for (size_t i = 0; i < size; i++) { + printf("%02X", *((unsigned char *) ptr + i)); + if ((i+1)%4 == 0) { + printf(" "); + } + + if((i+1)%32 == 0) { + printf("\n"); + } + } + printf("\n"); +} + +static MachOFile create_test_macho_file(void) { FILE *file = fopen(TEST_FILE, "wb"); if (!file) { LOG_ERROR("Error with fopen() on %s file", TEST_FILE); @@ -36,7 +50,7 @@ static void create_test_macho_file(void) { SectionHeader test_const_section = { .sectname = "__const", - .size = 2, // "hi" (including string literal) + .size = 2, // "hi" .offset = test_text_section.offset + test_text_section.size, }; @@ -49,6 +63,38 @@ static void create_test_macho_file(void) { .strsize = 2, }; + SectionInfo expected_text_section = { + .size = test_text_section.size, + .offset = test_text_section.offset, + }; + strcpy(expected_text_section.name, test_text_section.sectname); + + SectionInfo expected_const_section = { + .size = test_const_section.size, + .offset = test_const_section.offset, + }; + strcpy(expected_const_section.name, test_const_section.sectname); + + SectionInfo expected_symbol_table = { + .size = test_symtab_command.nsyms * sizeof(nList), + .offset = test_symtab_command.symoff, + }; + strcpy(expected_symbol_table.name, "__symbol_table"); + + SectionInfo expected_string_table = { + .size = test_symtab_command.strsize, + .offset = test_symtab_command.stroff, + }; + strcpy(expected_string_table.name, "__string_table"); + + SectionInfo expected_sections[4] = {expected_text_section, expected_const_section, expected_symbol_table, expected_string_table}; + + MachOFile expected = { + .machHeader = test_header, + .numSections = 4, + .sections = expected_sections, + }; + fwrite(&test_header, sizeof(MachOHeader), 1, file); fwrite(&test_text_segment, sizeof(SegmentLoadCommand), 1, file); fwrite(&test_text_section, sizeof(SectionHeader), 1, file); @@ -93,6 +139,8 @@ static void create_test_macho_file(void) { if (fclose(file) != 0) { LOG_ERROR("bad\n"); } + + return expected; } static void cleanup(void) { @@ -102,23 +150,31 @@ static void cleanup(void) { } } -static void test_read_macho_file(void) { +static void test_read_macho_file(MachOFile expected) { MachOFile test_macho_file; if(!read_macho_file(TEST_FILE, &test_macho_file)) { LOG_ERROR("Something in read_macho_file broke"); exit(EXIT_FAILURE); } - if (test_macho_file.machHeader.magic != MH_MAGIC_64) { - LOG_ERROR("Incorrect header magic value"); + if (memcmp(&test_macho_file.machHeader, &expected.machHeader, sizeof(MachOHeader)) != 0) { + LOG_ERROR("test_read_macho_file: read header is different than expected"); + exit(EXIT_FAILURE); } - if (test_macho_file.machHeader.ncmds != 2) { - LOG_ERROR("Incorrect header ncmds value"); + if (test_macho_file.numSections != expected.numSections) { + LOG_ERROR("test_read_macho_file: read number of sections is dfferent than expected"); + exit(EXIT_FAILURE); } - if (test_macho_file.machHeader.sizeofcmds != sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand)) { - LOG_ERROR("Incorrect header sizeofcmds value"); + if (memcmp(test_macho_file.sections, expected.sections, test_macho_file.numSections * sizeof(SectionInfo)) != 0) { + LOG_ERROR("test_read_macho_file: read section information is different than expected"); + printf("test:\n"); + print_hex(test_macho_file.sections, test_macho_file.numSections * sizeof(SectionInfo)); + printf("expected:\n"); + print_hex(expected.sections, expected.numSections * sizeof(SectionInfo)); + + print_macho_section_info(&test_macho_file); + print_macho_section_info(&expected); } - } static void test_free_macho_file(void) { @@ -139,8 +195,8 @@ static void test_find_macho_symbol_index(void) { int main(int argc, char *argv[]) { - create_test_macho_file(); - test_read_macho_file(); + MachOFile expected = create_test_macho_file(); + test_read_macho_file(expected); test_free_macho_file(); test_print_macho_section_info(); test_get_macho_section_data(); From a6f38907ff1bb9451659286ae34e49b4b3f77d8e Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 18 Jan 2024 10:26:59 -0800 Subject: [PATCH 06/36] get read macho file test working --- .../macho_parser/tests/macho_tests.c | 111 ++++++++++-------- 1 file changed, 64 insertions(+), 47 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index adcc72d7d6..4c8616614a 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -29,70 +29,50 @@ static MachOFile create_test_macho_file(void) { exit(EXIT_FAILURE); } + uint32_t header_sizeofcmds = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand); + uint32_t header_ncmds = 2; MachOHeader test_header = { .magic = MH_MAGIC_64, - .ncmds = 2, - .sizeofcmds = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand), + .ncmds = header_ncmds, + .sizeofcmds = header_sizeofcmds, }; + uint32_t text_segment_cmdsize = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader); + uint32_t text_segment_nsects = 2; SegmentLoadCommand test_text_segment = { .cmd = LC_SEGMENT_64, - .cmdsize = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader), + .cmdsize = text_segment_cmdsize, .segname = "__TEXT", - .nsects = 2, + .nsects = text_segment_nsects, }; + uint32_t text_section_offset = sizeof(MachOHeader) + sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand); + uint64_t text_section_size = 1; // {0xC3} SectionHeader test_text_section = { .sectname = "__text", - .size = 1, // {0xC3} - .offset = sizeof(MachOHeader) + sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand), + .size = text_section_size, + .offset = text_section_offset, }; + uint32_t const_section_offset = text_section_offset + text_section_size; + uint64_t const_section_size = 2; // "hi" SectionHeader test_const_section = { .sectname = "__const", - .size = 2, // "hi" - .offset = test_text_section.offset + test_text_section.size, + .size = const_section_size, + .offset = const_section_offset, }; + uint32_t symtab_command_symoff = const_section_offset + const_section_size; + uint32_t symtab_command_nsyms = 2; + uint32_t symtab_command_stroff = symtab_command_symoff + 2 * sizeof(nList); + uint32_t symtab_command_strsize = 2; SymtabLoadCommand test_symtab_command = { .cmd = LC_SYMTAB, .cmdsize = sizeof(SymtabLoadCommand), - .symoff = test_const_section.offset + test_const_section.size, - .nsyms = 2, - .stroff = test_const_section.offset + test_const_section.size + 2 * sizeof(nList), - .strsize = 2, - }; - - SectionInfo expected_text_section = { - .size = test_text_section.size, - .offset = test_text_section.offset, - }; - strcpy(expected_text_section.name, test_text_section.sectname); - - SectionInfo expected_const_section = { - .size = test_const_section.size, - .offset = test_const_section.offset, - }; - strcpy(expected_const_section.name, test_const_section.sectname); - - SectionInfo expected_symbol_table = { - .size = test_symtab_command.nsyms * sizeof(nList), - .offset = test_symtab_command.symoff, - }; - strcpy(expected_symbol_table.name, "__symbol_table"); - - SectionInfo expected_string_table = { - .size = test_symtab_command.strsize, - .offset = test_symtab_command.stroff, - }; - strcpy(expected_string_table.name, "__string_table"); - - SectionInfo expected_sections[4] = {expected_text_section, expected_const_section, expected_symbol_table, expected_string_table}; - - MachOFile expected = { - .machHeader = test_header, - .numSections = 4, - .sections = expected_sections, + .symoff = symtab_command_symoff, + .nsyms = symtab_command_nsyms, + .stroff = symtab_command_stroff, + .strsize = symtab_command_strsize, }; fwrite(&test_header, sizeof(MachOHeader), 1, file); @@ -140,6 +120,44 @@ static MachOFile create_test_macho_file(void) { LOG_ERROR("bad\n"); } + + SectionInfo expected_text_section = { + .name = "__text", + .size = text_section_size, + .offset = text_section_offset, + }; + + SectionInfo expected_const_section = { + .name = "__const", + .size = const_section_size, + .offset = const_section_offset, + }; + + SectionInfo expected_symbol_table = { + .name = "__symbol_table", + .size = symtab_command_nsyms * sizeof(nList), + .offset = symtab_command_symoff, + }; + + SectionInfo expected_string_table = { + .name = "__string_table", + .size = symtab_command_strsize, + .offset = symtab_command_stroff, + }; + + // SectionInfo expected_sections[4] = {expected_text_section, expected_const_section, expected_symbol_table, expected_string_table}; + SectionInfo *expected_sections = malloc(sizeof(SectionInfo) * 4); + expected_sections[0] = expected_text_section; + expected_sections[1] = expected_const_section; + expected_sections[2] = expected_symbol_table; + expected_sections[3] = expected_string_table; + + MachOFile expected = { + .machHeader = test_header, + .numSections = 4, + .sections = expected_sections, + }; + return expected; } @@ -171,9 +189,7 @@ static void test_read_macho_file(MachOFile expected) { print_hex(test_macho_file.sections, test_macho_file.numSections * sizeof(SectionInfo)); printf("expected:\n"); print_hex(expected.sections, expected.numSections * sizeof(SectionInfo)); - - print_macho_section_info(&test_macho_file); - print_macho_section_info(&expected); + exit(EXIT_FAILURE); } } @@ -202,6 +218,7 @@ int main(int argc, char *argv[]) { test_get_macho_section_data(); test_find_macho_symbol_index(); + free(expected); cleanup(); printf("All tests passed\n"); From bf18e420e0caa78ee7c98a99b3d7500f2722ee96 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 18 Jan 2024 11:04:43 -0800 Subject: [PATCH 07/36] rework macho free function --- .../inject_hash/macho_parser/macho_parser.c | 5 ++-- .../macho_parser/tests/macho_tests.c | 30 ++++++++----------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index d7b6833a61..0247c11946 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -69,10 +69,9 @@ int read_macho_file(const char *filename, MachOFile *macho) { } void free_macho_file(MachOFile *macho) { - for (uint32_t i = 0; i < macho->numSections; i++) { - free(macho->sections[i].name); - } free(macho->sections); + free(macho); + macho = NULL; } void print_macho_section_info(MachOFile *macho) { diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 4c8616614a..ec6d0c27c6 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -22,7 +22,7 @@ static void print_hex(const void *ptr, size_t size) { printf("\n"); } -static MachOFile create_test_macho_file(void) { +static MachOFile* create_test_macho_file(void) { FILE *file = fopen(TEST_FILE, "wb"); if (!file) { LOG_ERROR("Error with fopen() on %s file", TEST_FILE); @@ -152,11 +152,10 @@ static MachOFile create_test_macho_file(void) { expected_sections[2] = expected_symbol_table; expected_sections[3] = expected_string_table; - MachOFile expected = { - .machHeader = test_header, - .numSections = 4, - .sections = expected_sections, - }; + MachOFile *expected = malloc(sizeof(SectionInfo)); + expected->machHeader = test_header, + expected->numSections = 4, + expected->sections = expected_sections; return expected; } @@ -168,35 +167,31 @@ static void cleanup(void) { } } -static void test_read_macho_file(MachOFile expected) { +static void test_read_macho_file(MachOFile *expected) { MachOFile test_macho_file; if(!read_macho_file(TEST_FILE, &test_macho_file)) { LOG_ERROR("Something in read_macho_file broke"); exit(EXIT_FAILURE); } - if (memcmp(&test_macho_file.machHeader, &expected.machHeader, sizeof(MachOHeader)) != 0) { + if (memcmp(&test_macho_file.machHeader, &expected->machHeader, sizeof(MachOHeader)) != 0) { LOG_ERROR("test_read_macho_file: read header is different than expected"); exit(EXIT_FAILURE); } - if (test_macho_file.numSections != expected.numSections) { + if (test_macho_file.numSections != expected->numSections) { LOG_ERROR("test_read_macho_file: read number of sections is dfferent than expected"); exit(EXIT_FAILURE); } - if (memcmp(test_macho_file.sections, expected.sections, test_macho_file.numSections * sizeof(SectionInfo)) != 0) { + if (memcmp(test_macho_file.sections, expected->sections, test_macho_file.numSections * sizeof(SectionInfo)) != 0) { LOG_ERROR("test_read_macho_file: read section information is different than expected"); printf("test:\n"); print_hex(test_macho_file.sections, test_macho_file.numSections * sizeof(SectionInfo)); printf("expected:\n"); - print_hex(expected.sections, expected.numSections * sizeof(SectionInfo)); + print_hex(expected->sections, expected->numSections * sizeof(SectionInfo)); exit(EXIT_FAILURE); } } -static void test_free_macho_file(void) { - assert (1 == 1); -} - static void test_print_macho_section_info(void) { assert (1 == 1); } @@ -211,14 +206,13 @@ static void test_find_macho_symbol_index(void) { int main(int argc, char *argv[]) { - MachOFile expected = create_test_macho_file(); + MachOFile *expected = create_test_macho_file(); test_read_macho_file(expected); - test_free_macho_file(); test_print_macho_section_info(); test_get_macho_section_data(); test_find_macho_symbol_index(); - free(expected); + free_macho_file(expected); cleanup(); printf("All tests passed\n"); From 996a19e6ffed10cd20133d4459da6d2ecaff5dce Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 18 Jan 2024 11:09:16 -0800 Subject: [PATCH 08/36] remove unused macho print helper function --- util/fipstools/inject_hash/macho_parser/macho_parser.c | 8 -------- .../inject_hash/macho_parser/tests/macho_tests.c | 5 ----- 2 files changed, 13 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 0247c11946..9120529153 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -74,14 +74,6 @@ void free_macho_file(MachOFile *macho) { macho = NULL; } -void print_macho_section_info(MachOFile *macho) { - printf("Number of sections: %u\n", macho->numSections); - for (uint32_t i = 0; i < macho->numSections; i++) { - printf("Section: %s, Offset: %u, Size: %zu\n", macho->sections[i].name, - macho->sections[i].offset, macho->sections[i].size); - } -} - uint8_t* get_macho_section_data(char *filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset) { FILE *file = fopen(filename, "rb"); if (!file) { diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index ec6d0c27c6..19873b57cf 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -192,10 +192,6 @@ static void test_read_macho_file(MachOFile *expected) { } } -static void test_print_macho_section_info(void) { - assert (1 == 1); -} - static void test_get_macho_section_data(void) { assert (1 == 1); } @@ -208,7 +204,6 @@ int main(int argc, char *argv[]) { MachOFile *expected = create_test_macho_file(); test_read_macho_file(expected); - test_print_macho_section_info(); test_get_macho_section_data(); test_find_macho_symbol_index(); From 6b2b12d402822788d3148f1737413e4b831791f0 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 18 Jan 2024 11:47:39 -0800 Subject: [PATCH 09/36] implement get_macho_section_data test --- .../inject_hash/macho_parser/macho_parser.c | 4 +- .../inject_hash/macho_parser/macho_parser.h | 2 +- .../macho_parser/tests/macho_tests.c | 59 +++++++++++-------- 3 files changed, 37 insertions(+), 28 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 9120529153..040e2dff0e 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -74,7 +74,9 @@ void free_macho_file(MachOFile *macho) { macho = NULL; } -uint8_t* get_macho_section_data(char *filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset) { +// Takes a filename, MachOFile struct, the name of the section to get data for, and pointers to size & offset as input +// size and offset pointers are set to the size and offset of the section retrived in the file. +uint8_t* get_macho_section_data(const char *filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset) { FILE *file = fopen(filename, "rb"); if (!file) { LOG_ERROR("Error opening file %s", filename); diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index 74a97ac059..82e7922ff2 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -33,7 +33,7 @@ typedef struct { int read_macho_file(const char *filename, MachOFile *macho); void free_macho_file(MachOFile *macho); void print_macho_section_info(MachOFile *macho); -uint8_t* get_macho_section_data(char* filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset); +uint8_t* get_macho_section_data(const char* filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset); uint32_t find_macho_symbol_index(uint8_t *sectionData, size_t sectionSize, uint8_t *stringTableData, size_t stringTableSize, const char *symbolName, uint32_t *base); #endif diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 19873b57cf..b23971fd86 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -7,6 +7,7 @@ #include "../macho_parser.h" #define TEST_FILE "test_macho" +#define TEXT_DATA {0xC3} static void print_hex(const void *ptr, size_t size) { for (size_t i = 0; i < size; i++) { @@ -81,7 +82,7 @@ static MachOFile* create_test_macho_file(void) { fwrite(&test_const_section, sizeof(SectionHeader), 1, file); fwrite(&test_symtab_command, sizeof(SymtabLoadCommand), 1, file); - char test_text_data[] = {0xC3}; + char test_text_data[] = TEXT_DATA; char test_const_data[] = "hi"; fseek(file, test_text_section.offset, SEEK_SET); @@ -90,31 +91,29 @@ static MachOFile* create_test_macho_file(void) { fseek(file, test_const_section.offset, SEEK_SET); fwrite(test_const_data, sizeof(test_const_data), 1, file); - // Leave out symbol and string tables for now - - // nList symbol1 = { - // .n_un = {.n_strx = 1}, // Index into the string table - // .n_type = N_TEXT, - // .n_sect = 1, - // .n_desc = 0, - // .n_value = 0x100000000, // Address of the symbol - // }; + nList symbol1 = { + .n_un = {.n_strx = 1}, // Index into the string table + .n_type = 0, + .n_sect = 1, + .n_desc = 0, + .n_value = 0x100000000, // Address of the symbol + }; - // nList symbol2 = { - // .n_un = {.n_strx = 15}, // Index into the string table - // .n_type = N_DATA, - // .n_sect = 2, - // .n_desc = 0, - // .n_value = 0x100000000 + sizeof(test_text_data), // Address of the symbol - // }; + nList symbol2 = { + .n_un = {.n_strx = 15}, // Index into the string table + .n_type = 0, + .n_sect = 2, + .n_desc = 0, + .n_value = 0x100000000 + sizeof(test_text_data), // Address of the symbol + }; - // fwrite(&symbol1, sizeof(struct nlist_64), 1, file); - // fwrite(&symbol2, sizeof(struct nlist_64), 1, file); + fwrite(&symbol1, sizeof(struct nlist_64), 1, file); + fwrite(&symbol2, sizeof(struct nlist_64), 1, file); - // // Write the string table - // char string_table[] = "\0__text\0__const\0symbol1\0symbol2"; - // fseek(file, symtab_cmd.stroff, SEEK_SET); - // fwrite(string_table, sizeof(string_table), 1, file); + // Write the string table + char string_table[] = "\0__text\0__const\0symbol1\0symbol2"; + fseek(file, symtab_command_stroff, SEEK_SET); + fwrite(string_table, sizeof(string_table), 1, file); if (fclose(file) != 0) { LOG_ERROR("bad\n"); @@ -192,8 +191,16 @@ static void test_read_macho_file(MachOFile *expected) { } } -static void test_get_macho_section_data(void) { - assert (1 == 1); +static void test_get_macho_section_data(MachOFile *expected) { + uint8_t *text_section = NULL; + size_t text_section_size; + uint32_t text_section_offset; + char expected_text_section[] = TEXT_DATA; + + text_section = get_macho_section_data(TEST_FILE, expected, "__text", &text_section_size, &text_section_offset); + if (memcmp(text_section, expected_text_section, text_section_size) != 0) { + LOG_ERROR("text section is not equal to what was expected"); + } } static void test_find_macho_symbol_index(void) { @@ -204,7 +211,7 @@ int main(int argc, char *argv[]) { MachOFile *expected = create_test_macho_file(); test_read_macho_file(expected); - test_get_macho_section_data(); + test_get_macho_section_data(expected); test_find_macho_symbol_index(); free_macho_file(expected); From 2df5ec79f5778045b49f3ee6ff3eb96fcaf9eb48 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Fri, 19 Jan 2024 13:22:52 -0800 Subject: [PATCH 10/36] fix symbol table not being read correctly sometimes --- .../macho_parser/tests/macho_tests.c | 91 ++++++++++++++++--- 1 file changed, 78 insertions(+), 13 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index b23971fd86..78830a0912 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -7,7 +7,9 @@ #include "../macho_parser.h" #define TEST_FILE "test_macho" + #define TEXT_DATA {0xC3} +#define CONST_DATA "hi" static void print_hex(const void *ptr, size_t size) { for (size_t i = 0; i < size; i++) { @@ -23,6 +25,38 @@ static void print_hex(const void *ptr, size_t size) { printf("\n"); } +static void print_hex_file(const char* filename) { + FILE *file = fopen(filename, "rb"); + + if (file == NULL) { + perror("Error opening file"); + return; + } + + int byte; + size_t count = 0; + + while ((byte = fgetc(file)) != EOF) { + printf("%02X", byte); + count++; + + if (count % 4 == 0) { + printf(" "); + } + + if (count % 32 == 0) { + printf("\n"); + } + } + + // Check if the last line is incomplete + if (count % 32 != 0) { + printf("\n"); + } + + fclose(file); +} + static MachOFile* create_test_macho_file(void) { FILE *file = fopen(TEST_FILE, "wb"); if (!file) { @@ -65,8 +99,8 @@ static MachOFile* create_test_macho_file(void) { uint32_t symtab_command_symoff = const_section_offset + const_section_size; uint32_t symtab_command_nsyms = 2; - uint32_t symtab_command_stroff = symtab_command_symoff + 2 * sizeof(nList); - uint32_t symtab_command_strsize = 2; + uint32_t symtab_command_stroff = symtab_command_symoff + symtab_command_nsyms * sizeof(nList); + uint32_t symtab_command_strsize = 32; SymtabLoadCommand test_symtab_command = { .cmd = LC_SYMTAB, .cmdsize = sizeof(SymtabLoadCommand), @@ -83,7 +117,7 @@ static MachOFile* create_test_macho_file(void) { fwrite(&test_symtab_command, sizeof(SymtabLoadCommand), 1, file); char test_text_data[] = TEXT_DATA; - char test_const_data[] = "hi"; + char test_const_data[] = CONST_DATA; fseek(file, test_text_section.offset, SEEK_SET); fwrite(test_text_data, sizeof(test_text_data), 1, file); @@ -92,7 +126,7 @@ static MachOFile* create_test_macho_file(void) { fwrite(test_const_data, sizeof(test_const_data), 1, file); nList symbol1 = { - .n_un = {.n_strx = 1}, // Index into the string table + .n_un = {.n_strx = 15}, // Index into the string table .n_type = 0, .n_sect = 1, .n_desc = 0, @@ -100,15 +134,16 @@ static MachOFile* create_test_macho_file(void) { }; nList symbol2 = { - .n_un = {.n_strx = 15}, // Index into the string table + .n_un = {.n_strx = 23}, // Index into the string table .n_type = 0, .n_sect = 2, .n_desc = 0, .n_value = 0x100000000 + sizeof(test_text_data), // Address of the symbol }; - fwrite(&symbol1, sizeof(struct nlist_64), 1, file); - fwrite(&symbol2, sizeof(struct nlist_64), 1, file); + fseek(file, symtab_command_symoff, SEEK_SET); + fwrite(&symbol1, sizeof(nList), 1, file); + fwrite(&symbol2, sizeof(nList), 1, file); // Write the string table char string_table[] = "\0__text\0__const\0symbol1\0symbol2"; @@ -116,10 +151,9 @@ static MachOFile* create_test_macho_file(void) { fwrite(string_table, sizeof(string_table), 1, file); if (fclose(file) != 0) { - LOG_ERROR("bad\n"); + LOG_ERROR("Error closing file\n"); } - SectionInfo expected_text_section = { .name = "__text", .size = text_section_size, @@ -144,14 +178,13 @@ static MachOFile* create_test_macho_file(void) { .offset = symtab_command_stroff, }; - // SectionInfo expected_sections[4] = {expected_text_section, expected_const_section, expected_symbol_table, expected_string_table}; SectionInfo *expected_sections = malloc(sizeof(SectionInfo) * 4); expected_sections[0] = expected_text_section; expected_sections[1] = expected_const_section; expected_sections[2] = expected_symbol_table; expected_sections[3] = expected_string_table; - MachOFile *expected = malloc(sizeof(SectionInfo)); + MachOFile *expected = malloc(sizeof(MachOFile)); expected->machHeader = test_header, expected->numSections = 4, expected->sections = expected_sections; @@ -194,12 +227,43 @@ static void test_read_macho_file(MachOFile *expected) { static void test_get_macho_section_data(MachOFile *expected) { uint8_t *text_section = NULL; size_t text_section_size; - uint32_t text_section_offset; char expected_text_section[] = TEXT_DATA; - text_section = get_macho_section_data(TEST_FILE, expected, "__text", &text_section_size, &text_section_offset); + uint8_t *const_section = NULL; + size_t const_section_size; + char expected_const_section[] = CONST_DATA; + + uint8_t *symbol_table = NULL; + size_t symbol_table_size; + char expected_symbol_table[] = "something?"; + + uint8_t *string_table = NULL; + size_t string_table_size; + char expected_string_table[] = "\0__text\0__const\0symbol1\0symbol2"; + + text_section = get_macho_section_data(TEST_FILE, expected, "__text", &text_section_size, NULL); if (memcmp(text_section, expected_text_section, text_section_size) != 0) { LOG_ERROR("text section is not equal to what was expected"); + exit(EXIT_FAILURE); + } + + const_section = get_macho_section_data(TEST_FILE, expected, "__const", &const_section_size, NULL); + if (memcmp(const_section, expected_const_section, const_section_size) != 0) { + LOG_ERROR("const section is not equal to what was expected"); + exit(EXIT_FAILURE); + } + + // Something about calling this function on the symbol table causes something to segfault occasionally + symbol_table = get_macho_section_data(TEST_FILE, expected, "__symbol_table", &symbol_table_size, NULL); + if (memcmp(symbol_table, expected_symbol_table, symbol_table_size) != 0) { + LOG_ERROR("symbol table is not equal to what was expected"); + // exit(EXIT_FAILURE); until we get this test working + } + + string_table = get_macho_section_data(TEST_FILE, expected, "__string_table", &string_table_size, NULL); + if (memcmp(string_table, expected_string_table, string_table_size) != 0) { + LOG_ERROR("string table is not equal to what was expected"); + exit(EXIT_FAILURE); } } @@ -210,6 +274,7 @@ static void test_find_macho_symbol_index(void) { int main(int argc, char *argv[]) { MachOFile *expected = create_test_macho_file(); + print_hex_file(TEST_FILE); // needs to be run after the file is created test_read_macho_file(expected); test_get_macho_section_data(expected); test_find_macho_symbol_index(); From d27dfa0be42de4ac5f846d67fc8ba6846882f65e Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Fri, 19 Jan 2024 13:37:33 -0800 Subject: [PATCH 11/36] add working symbol table test --- .../macho_parser/tests/macho_tests.c | 64 +++++++++++-------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 78830a0912..37f828ae9e 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -11,6 +11,8 @@ #define TEXT_DATA {0xC3} #define CONST_DATA "hi" +#define NUM_SYMS 2 + static void print_hex(const void *ptr, size_t size) { for (size_t i = 0; i < size; i++) { printf("%02X", *((unsigned char *) ptr + i)); @@ -57,7 +59,13 @@ static void print_hex_file(const char* filename) { fclose(file); } -static MachOFile* create_test_macho_file(void) { +static void create_test_macho_file(MachOFile *macho, nList *symbol_table) { + + if (macho == NULL || symbol_table == NULL) { + LOG_ERROR("macho and symbol_table must be allocated"); + exit(EXIT_FAILURE); + } + FILE *file = fopen(TEST_FILE, "wb"); if (!file) { LOG_ERROR("Error with fopen() on %s file", TEST_FILE); @@ -98,14 +106,13 @@ static MachOFile* create_test_macho_file(void) { }; uint32_t symtab_command_symoff = const_section_offset + const_section_size; - uint32_t symtab_command_nsyms = 2; - uint32_t symtab_command_stroff = symtab_command_symoff + symtab_command_nsyms * sizeof(nList); + uint32_t symtab_command_stroff = symtab_command_symoff + NUM_SYMS * sizeof(nList); uint32_t symtab_command_strsize = 32; SymtabLoadCommand test_symtab_command = { .cmd = LC_SYMTAB, .cmdsize = sizeof(SymtabLoadCommand), .symoff = symtab_command_symoff, - .nsyms = symtab_command_nsyms, + .nsyms = NUM_SYMS, .stroff = symtab_command_stroff, .strsize = symtab_command_strsize, }; @@ -168,7 +175,7 @@ static MachOFile* create_test_macho_file(void) { SectionInfo expected_symbol_table = { .name = "__symbol_table", - .size = symtab_command_nsyms * sizeof(nList), + .size = NUM_SYMS * sizeof(nList), .offset = symtab_command_symoff, }; @@ -184,12 +191,12 @@ static MachOFile* create_test_macho_file(void) { expected_sections[2] = expected_symbol_table; expected_sections[3] = expected_string_table; - MachOFile *expected = malloc(sizeof(MachOFile)); - expected->machHeader = test_header, - expected->numSections = 4, - expected->sections = expected_sections; + macho->machHeader = test_header, + macho->numSections = 4, + macho->sections = expected_sections; - return expected; + symbol_table[0] = symbol1; + symbol_table[1] = symbol2; } static void cleanup(void) { @@ -199,32 +206,32 @@ static void cleanup(void) { } } -static void test_read_macho_file(MachOFile *expected) { +static void test_read_macho_file(MachOFile *expected_macho) { MachOFile test_macho_file; if(!read_macho_file(TEST_FILE, &test_macho_file)) { LOG_ERROR("Something in read_macho_file broke"); exit(EXIT_FAILURE); } - if (memcmp(&test_macho_file.machHeader, &expected->machHeader, sizeof(MachOHeader)) != 0) { + if (memcmp(&test_macho_file.machHeader, &expected_macho->machHeader, sizeof(MachOHeader)) != 0) { LOG_ERROR("test_read_macho_file: read header is different than expected"); exit(EXIT_FAILURE); } - if (test_macho_file.numSections != expected->numSections) { + if (test_macho_file.numSections != expected_macho->numSections) { LOG_ERROR("test_read_macho_file: read number of sections is dfferent than expected"); exit(EXIT_FAILURE); } - if (memcmp(test_macho_file.sections, expected->sections, test_macho_file.numSections * sizeof(SectionInfo)) != 0) { + if (memcmp(test_macho_file.sections, expected_macho->sections, test_macho_file.numSections * sizeof(SectionInfo)) != 0) { LOG_ERROR("test_read_macho_file: read section information is different than expected"); printf("test:\n"); print_hex(test_macho_file.sections, test_macho_file.numSections * sizeof(SectionInfo)); printf("expected:\n"); - print_hex(expected->sections, expected->numSections * sizeof(SectionInfo)); + print_hex(expected_macho->sections, expected_macho->numSections * sizeof(SectionInfo)); exit(EXIT_FAILURE); } } -static void test_get_macho_section_data(MachOFile *expected) { +static void test_get_macho_section_data(MachOFile *expected_macho, nList* expected_symtab) { uint8_t *text_section = NULL; size_t text_section_size; char expected_text_section[] = TEXT_DATA; @@ -235,32 +242,31 @@ static void test_get_macho_section_data(MachOFile *expected) { uint8_t *symbol_table = NULL; size_t symbol_table_size; - char expected_symbol_table[] = "something?"; uint8_t *string_table = NULL; size_t string_table_size; char expected_string_table[] = "\0__text\0__const\0symbol1\0symbol2"; - text_section = get_macho_section_data(TEST_FILE, expected, "__text", &text_section_size, NULL); + text_section = get_macho_section_data(TEST_FILE, expected_macho, "__text", &text_section_size, NULL); if (memcmp(text_section, expected_text_section, text_section_size) != 0) { LOG_ERROR("text section is not equal to what was expected"); exit(EXIT_FAILURE); } - const_section = get_macho_section_data(TEST_FILE, expected, "__const", &const_section_size, NULL); + const_section = get_macho_section_data(TEST_FILE, expected_macho, "__const", &const_section_size, NULL); if (memcmp(const_section, expected_const_section, const_section_size) != 0) { LOG_ERROR("const section is not equal to what was expected"); exit(EXIT_FAILURE); } // Something about calling this function on the symbol table causes something to segfault occasionally - symbol_table = get_macho_section_data(TEST_FILE, expected, "__symbol_table", &symbol_table_size, NULL); - if (memcmp(symbol_table, expected_symbol_table, symbol_table_size) != 0) { + symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); + if (memcmp(symbol_table, expected_symtab, symbol_table_size) != 0) { LOG_ERROR("symbol table is not equal to what was expected"); - // exit(EXIT_FAILURE); until we get this test working + exit(EXIT_FAILURE); } - string_table = get_macho_section_data(TEST_FILE, expected, "__string_table", &string_table_size, NULL); + string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); if (memcmp(string_table, expected_string_table, string_table_size) != 0) { LOG_ERROR("string table is not equal to what was expected"); exit(EXIT_FAILURE); @@ -273,13 +279,17 @@ static void test_find_macho_symbol_index(void) { int main(int argc, char *argv[]) { - MachOFile *expected = create_test_macho_file(); + MachOFile *expected_macho = malloc(sizeof(MachOFile)); + nList *expected_symtab = malloc(NUM_SYMS * sizeof(nList)); + + create_test_macho_file(expected_macho, expected_symtab); print_hex_file(TEST_FILE); // needs to be run after the file is created - test_read_macho_file(expected); - test_get_macho_section_data(expected); + test_read_macho_file(expected_macho); + test_get_macho_section_data(expected_macho, expected_symtab); test_find_macho_symbol_index(); - free_macho_file(expected); + free_macho_file(expected_macho); + free(expected_symtab); cleanup(); printf("All tests passed\n"); From 166ed739bcf5b0497c3a5fad05e737a3ce97b479 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Fri, 19 Jan 2024 13:54:43 -0800 Subject: [PATCH 12/36] refactor string table --- .../macho_parser/tests/macho_tests.c | 49 +++---------------- 1 file changed, 7 insertions(+), 42 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 37f828ae9e..3a7a441d38 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -27,40 +27,7 @@ static void print_hex(const void *ptr, size_t size) { printf("\n"); } -static void print_hex_file(const char* filename) { - FILE *file = fopen(filename, "rb"); - - if (file == NULL) { - perror("Error opening file"); - return; - } - - int byte; - size_t count = 0; - - while ((byte = fgetc(file)) != EOF) { - printf("%02X", byte); - count++; - - if (count % 4 == 0) { - printf(" "); - } - - if (count % 32 == 0) { - printf("\n"); - } - } - - // Check if the last line is incomplete - if (count % 32 != 0) { - printf("\n"); - } - - fclose(file); -} - -static void create_test_macho_file(MachOFile *macho, nList *symbol_table) { - +static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const char* string_table) { if (macho == NULL || symbol_table == NULL) { LOG_ERROR("macho and symbol_table must be allocated"); exit(EXIT_FAILURE); @@ -153,9 +120,8 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table) { fwrite(&symbol2, sizeof(nList), 1, file); // Write the string table - char string_table[] = "\0__text\0__const\0symbol1\0symbol2"; fseek(file, symtab_command_stroff, SEEK_SET); - fwrite(string_table, sizeof(string_table), 1, file); + fwrite(string_table, symtab_command_strsize, 1, file); if (fclose(file) != 0) { LOG_ERROR("Error closing file\n"); @@ -231,7 +197,7 @@ static void test_read_macho_file(MachOFile *expected_macho) { } } -static void test_get_macho_section_data(MachOFile *expected_macho, nList* expected_symtab) { +static void test_get_macho_section_data(MachOFile *expected_macho, nList* expected_symtab, const char* expected_strtab) { uint8_t *text_section = NULL; size_t text_section_size; char expected_text_section[] = TEXT_DATA; @@ -245,7 +211,6 @@ static void test_get_macho_section_data(MachOFile *expected_macho, nList* expect uint8_t *string_table = NULL; size_t string_table_size; - char expected_string_table[] = "\0__text\0__const\0symbol1\0symbol2"; text_section = get_macho_section_data(TEST_FILE, expected_macho, "__text", &text_section_size, NULL); if (memcmp(text_section, expected_text_section, text_section_size) != 0) { @@ -267,7 +232,7 @@ static void test_get_macho_section_data(MachOFile *expected_macho, nList* expect } string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); - if (memcmp(string_table, expected_string_table, string_table_size) != 0) { + if (memcmp(string_table, expected_strtab, string_table_size) != 0) { LOG_ERROR("string table is not equal to what was expected"); exit(EXIT_FAILURE); } @@ -281,11 +246,11 @@ int main(int argc, char *argv[]) { MachOFile *expected_macho = malloc(sizeof(MachOFile)); nList *expected_symtab = malloc(NUM_SYMS * sizeof(nList)); + char expected_strtab[] = "__text\0__const\0symbol1\0symbol2\0"; - create_test_macho_file(expected_macho, expected_symtab); - print_hex_file(TEST_FILE); // needs to be run after the file is created + create_test_macho_file(expected_macho, expected_symtab, expected_strtab); test_read_macho_file(expected_macho); - test_get_macho_section_data(expected_macho, expected_symtab); + test_get_macho_section_data(expected_macho, expected_symtab, expected_strtab); test_find_macho_symbol_index(); free_macho_file(expected_macho); From cff1be91beb61faa6e996461a0c36065ac8d3d87 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Mon, 22 Jan 2024 08:04:15 -0800 Subject: [PATCH 13/36] read index correctly in test --- .../inject_hash/macho_parser/macho_parser.c | 4 ++++ .../macho_parser/tests/macho_tests.c | 22 ++++++++++++++----- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 040e2dff0e..796e8d5e78 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -136,6 +136,10 @@ uint32_t find_macho_symbol_index(uint8_t *symbolTableData, size_t symbolTableSiz } } + if (!found) { + LOG_ERROR("Requested symbol %s not found", symbolName); + } + free(stringTable); if (base) { index = index - *base; diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 3a7a441d38..2664515e7c 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -104,7 +104,7 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const .n_type = 0, .n_sect = 1, .n_desc = 0, - .n_value = 0x100000000, // Address of the symbol + .n_value = 15, // Address of the symbol }; nList symbol2 = { @@ -112,7 +112,7 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const .n_type = 0, .n_sect = 2, .n_desc = 0, - .n_value = 0x100000000 + sizeof(test_text_data), // Address of the symbol + .n_value = 23, // Address of the symbol }; fseek(file, symtab_command_symoff, SEEK_SET); @@ -238,8 +238,20 @@ static void test_get_macho_section_data(MachOFile *expected_macho, nList* expect } } -static void test_find_macho_symbol_index(void) { - assert (1 == 1); +static void test_find_macho_symbol_index(MachOFile *expected_macho, const char *expected_strtab) { + uint8_t *symbol_table = NULL; + size_t symbol_table_size; + + uint8_t *string_table = NULL; + size_t string_table_size; + + symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); + string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); + + uint32_t symbol1_index; + symbol1_index = find_macho_symbol_index(symbol_table, symbol_table_size, string_table, string_table_size, "symbol1", NULL); + + printf("%u\n", symbol1_index); } int main(int argc, char *argv[]) { @@ -251,7 +263,7 @@ int main(int argc, char *argv[]) { create_test_macho_file(expected_macho, expected_symtab, expected_strtab); test_read_macho_file(expected_macho); test_get_macho_section_data(expected_macho, expected_symtab, expected_strtab); - test_find_macho_symbol_index(); + test_find_macho_symbol_index(expected_macho, expected_strtab); free_macho_file(expected_macho); free(expected_symtab); From 2195181ea62182f03c2839f8209529fd84e27410 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 25 Jan 2024 09:05:14 -0800 Subject: [PATCH 14/36] remove more camelcase --- .../inject_hash/macho_parser/macho_parser.c | 41 +++++----- .../inject_hash/macho_parser/macho_parser.h | 32 ++++---- .../macho_parser/tests/macho_tests.c | 78 +++++++++---------- 3 files changed, 76 insertions(+), 75 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 796e8d5e78..f74d9c77e3 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -8,38 +8,39 @@ /** * TODOs - * use goto for cleaner exits - * make all variable and function names snake case + * use goto for cleaner exits in this and tests (if necessary) + * make all variable and function names snake case in all files + * finish tests */ // Documentation for the Mach-O structs can be found in macho-o/loader.h and mach-o/nlist.h -int read_macho_file(const char *filename, MachOFile *macho) { +int read_macho_file(const char *filename, machofile *macho) { FILE *file = fopen(filename, "rb"); if (!file) { LOG_ERROR("Error opening file %s", filename); return 0; } - fread(&macho->machHeader, sizeof(MachOHeader), 1, file); - if(macho->machHeader.magic != MH_MAGIC_64) { + fread(&macho->macho_header, sizeof(macho_header), 1, file); + if(macho->macho_header.magic != MH_MAGIC_64) { LOG_ERROR("File is not a 64-bit Mach-O file"); return 0; } - LoadCommand *load_commands = malloc(macho->machHeader.sizeofcmds); - fread(load_commands, macho->machHeader.sizeofcmds, 1, file); + load_cmd *load_commands = malloc(macho->macho_header.sizeofcmds); + fread(load_commands, macho->macho_header.sizeofcmds, 1, file); // We're only looking for __text, __const in the __TEXT segment, and the string & symbol tables - macho->numSections = 4; - macho->sections = malloc(macho->numSections * sizeof(SectionInfo)); + macho->num_sections = 4; + macho->sections = malloc(macho->num_sections * sizeof(section_info)); // Iterate through load commands again to populate section information uint32_t sectionIndex = 0; - for (uint32_t i = 0; i < macho->machHeader.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { + for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { if (load_commands[i].cmd == LC_SEG) { - SegmentLoadCommand *segment = (SegmentLoadCommand *)&load_commands[i]; + segment_load_cmd *segment = (segment_load_cmd *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { - SectionHeader *sections = (SectionHeader *)&segment[1]; + section *sections = (section *)&segment[1]; for (uint32_t j = 0; j < segment->nsects; j++) { if (strcmp(sections[j].sectname, "__text") == 0 || strcmp(sections[j].sectname, "__const") == 0) { macho->sections[sectionIndex].offset = sections[j].offset; @@ -50,9 +51,9 @@ int read_macho_file(const char *filename, MachOFile *macho) { } } } else if (load_commands[i].cmd == LC_SYMTAB) { - SymtabLoadCommand *symtab = (SymtabLoadCommand *)&load_commands[i]; + symtab_load_cmd *symtab = (symtab_load_cmd *)&load_commands[i]; macho->sections[sectionIndex].offset = symtab->symoff; - macho->sections[sectionIndex].size = symtab->nsyms * sizeof(nList); + macho->sections[sectionIndex].size = symtab->nsyms * sizeof(symbol_info); strcpy(macho->sections[sectionIndex].name, "__symbol_table"); sectionIndex++; macho->sections[sectionIndex].offset = symtab->stroff; @@ -68,21 +69,21 @@ int read_macho_file(const char *filename, MachOFile *macho) { return 1; } -void free_macho_file(MachOFile *macho) { +void free_macho_file(machofile *macho) { free(macho->sections); free(macho); macho = NULL; } -// Takes a filename, MachOFile struct, the name of the section to get data for, and pointers to size & offset as input +// Takes a filename, machofile struct, the name of the section to get data for, and pointers to size & offset as input // size and offset pointers are set to the size and offset of the section retrived in the file. -uint8_t* get_macho_section_data(const char *filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset) { +uint8_t* get_macho_section_data(const char *filename, machofile *macho, const char *sectionName, size_t *size, uint32_t *offset) { FILE *file = fopen(filename, "rb"); if (!file) { LOG_ERROR("Error opening file %s", filename); return NULL; } - for (uint32_t i = 0; i < macho->numSections; i++) { + for (uint32_t i = 0; i < macho->num_sections; i++) { if (strcmp(macho->sections[i].name, sectionName) == 0) { uint8_t *sectionData = malloc(macho->sections[i].size); if (!sectionData) { @@ -122,8 +123,8 @@ uint32_t find_macho_symbol_index(uint8_t *symbolTableData, size_t symbolTableSiz int found = 0; uint32_t index = 0; - for (uint32_t i = 0; i < symbolTableSize / sizeof(nList); i++) { - nList *symbol = (nList *)(symbolTableData + i * sizeof(nList)); + for (uint32_t i = 0; i < symbolTableSize / sizeof(symbol_info); i++) { + symbol_info *symbol = (symbol_info *)(symbolTableData + i * sizeof(symbol_info)); if (strcmp(symbolName, &stringTable[symbol->n_un.n_strx]) == 0) { if (!found) { index = symbol->n_value; diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index 82e7922ff2..19b7ba77c6 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -11,29 +11,29 @@ typedef struct { char name[16]; size_t size; uint32_t offset; -} SectionInfo; +} section_info; // Since we only support 64-bit architectures on Apple, we don't need to account for any of the 32-bit structures #define LC_SEG LC_SEGMENT_64 #define BIT_MODIFIER 8 -typedef struct mach_header_64 MachOHeader; -typedef struct load_command LoadCommand; -typedef struct segment_command_64 SegmentLoadCommand; -typedef struct symtab_command SymtabLoadCommand; -typedef struct section_64 SectionHeader; -typedef struct nlist_64 nList; +typedef struct mach_header_64 macho_header; +typedef struct load_command load_cmd; +typedef struct segment_command_64 segment_load_cmd; +typedef struct symtab_command symtab_load_cmd; +typedef struct section_64 section; +typedef struct nlist_64 symbol_info; typedef struct { - MachOHeader machHeader; - SectionInfo *sections; - uint32_t numSections; -} MachOFile; - -int read_macho_file(const char *filename, MachOFile *macho); -void free_macho_file(MachOFile *macho); -void print_macho_section_info(MachOFile *macho); -uint8_t* get_macho_section_data(const char* filename, MachOFile *macho, const char *sectionName, size_t *size, uint32_t *offset); + macho_header macho_header; + section_info *sections; + uint32_t num_sections; +} machofile; + +int read_macho_file(const char *filename, machofile *macho); +void free_macho_file(machofile *macho); +void print_macho_section_info(machofile *macho); +uint8_t* get_macho_section_data(const char* filename, machofile *macho, const char *sectionName, size_t *size, uint32_t *offset); uint32_t find_macho_symbol_index(uint8_t *sectionData, size_t sectionSize, uint8_t *stringTableData, size_t stringTableSize, const char *symbolName, uint32_t *base); #endif diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 2664515e7c..940405007b 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -27,7 +27,7 @@ static void print_hex(const void *ptr, size_t size) { printf("\n"); } -static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const char* string_table) { +static void create_test_macho_file(machofile *macho, symbol_info *symbol_table, const char* string_table) { if (macho == NULL || symbol_table == NULL) { LOG_ERROR("macho and symbol_table must be allocated"); exit(EXIT_FAILURE); @@ -39,26 +39,26 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const exit(EXIT_FAILURE); } - uint32_t header_sizeofcmds = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand); + uint32_t header_sizeofcmds = sizeof(segment_load_cmd) + 2 * sizeof(section) + sizeof(symtab_load_cmd); uint32_t header_ncmds = 2; - MachOHeader test_header = { + macho_header test_header = { .magic = MH_MAGIC_64, .ncmds = header_ncmds, .sizeofcmds = header_sizeofcmds, }; - uint32_t text_segment_cmdsize = sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader); + uint32_t text_segment_cmdsize = sizeof(segment_load_cmd) + 2 * sizeof(section); uint32_t text_segment_nsects = 2; - SegmentLoadCommand test_text_segment = { + segment_load_cmd test_text_segment = { .cmd = LC_SEGMENT_64, .cmdsize = text_segment_cmdsize, .segname = "__TEXT", .nsects = text_segment_nsects, }; - uint32_t text_section_offset = sizeof(MachOHeader) + sizeof(SegmentLoadCommand) + 2 * sizeof(SectionHeader) + sizeof(SymtabLoadCommand); + uint32_t text_section_offset = sizeof(macho_header) + sizeof(segment_load_cmd) + 2 * sizeof(section) + sizeof(symtab_load_cmd); uint64_t text_section_size = 1; // {0xC3} - SectionHeader test_text_section = { + section test_text_section = { .sectname = "__text", .size = text_section_size, .offset = text_section_offset, @@ -66,29 +66,29 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const uint32_t const_section_offset = text_section_offset + text_section_size; uint64_t const_section_size = 2; // "hi" - SectionHeader test_const_section = { + section test_const_section = { .sectname = "__const", .size = const_section_size, .offset = const_section_offset, }; uint32_t symtab_command_symoff = const_section_offset + const_section_size; - uint32_t symtab_command_stroff = symtab_command_symoff + NUM_SYMS * sizeof(nList); + uint32_t symtab_command_stroff = symtab_command_symoff + NUM_SYMS * sizeof(symbol_info); uint32_t symtab_command_strsize = 32; - SymtabLoadCommand test_symtab_command = { + symtab_load_cmd test_symtab_command = { .cmd = LC_SYMTAB, - .cmdsize = sizeof(SymtabLoadCommand), + .cmdsize = sizeof(symtab_load_cmd), .symoff = symtab_command_symoff, .nsyms = NUM_SYMS, .stroff = symtab_command_stroff, .strsize = symtab_command_strsize, }; - fwrite(&test_header, sizeof(MachOHeader), 1, file); - fwrite(&test_text_segment, sizeof(SegmentLoadCommand), 1, file); - fwrite(&test_text_section, sizeof(SectionHeader), 1, file); - fwrite(&test_const_section, sizeof(SectionHeader), 1, file); - fwrite(&test_symtab_command, sizeof(SymtabLoadCommand), 1, file); + fwrite(&test_header, sizeof(macho_header), 1, file); + fwrite(&test_text_segment, sizeof(segment_load_cmd), 1, file); + fwrite(&test_text_section, sizeof(section), 1, file); + fwrite(&test_const_section, sizeof(section), 1, file); + fwrite(&test_symtab_command, sizeof(symtab_load_cmd), 1, file); char test_text_data[] = TEXT_DATA; char test_const_data[] = CONST_DATA; @@ -99,7 +99,7 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const fseek(file, test_const_section.offset, SEEK_SET); fwrite(test_const_data, sizeof(test_const_data), 1, file); - nList symbol1 = { + symbol_info symbol1 = { .n_un = {.n_strx = 15}, // Index into the string table .n_type = 0, .n_sect = 1, @@ -107,7 +107,7 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const .n_value = 15, // Address of the symbol }; - nList symbol2 = { + symbol_info symbol2 = { .n_un = {.n_strx = 23}, // Index into the string table .n_type = 0, .n_sect = 2, @@ -116,8 +116,8 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const }; fseek(file, symtab_command_symoff, SEEK_SET); - fwrite(&symbol1, sizeof(nList), 1, file); - fwrite(&symbol2, sizeof(nList), 1, file); + fwrite(&symbol1, sizeof(symbol_info), 1, file); + fwrite(&symbol2, sizeof(symbol_info), 1, file); // Write the string table fseek(file, symtab_command_stroff, SEEK_SET); @@ -127,38 +127,38 @@ static void create_test_macho_file(MachOFile *macho, nList *symbol_table, const LOG_ERROR("Error closing file\n"); } - SectionInfo expected_text_section = { + section_info expected_text_section = { .name = "__text", .size = text_section_size, .offset = text_section_offset, }; - SectionInfo expected_const_section = { + section_info expected_const_section = { .name = "__const", .size = const_section_size, .offset = const_section_offset, }; - SectionInfo expected_symbol_table = { + section_info expected_symbol_table = { .name = "__symbol_table", - .size = NUM_SYMS * sizeof(nList), + .size = NUM_SYMS * sizeof(symbol_info), .offset = symtab_command_symoff, }; - SectionInfo expected_string_table = { + section_info expected_string_table = { .name = "__string_table", .size = symtab_command_strsize, .offset = symtab_command_stroff, }; - SectionInfo *expected_sections = malloc(sizeof(SectionInfo) * 4); + section_info *expected_sections = malloc(sizeof(section_info) * 4); expected_sections[0] = expected_text_section; expected_sections[1] = expected_const_section; expected_sections[2] = expected_symbol_table; expected_sections[3] = expected_string_table; - macho->machHeader = test_header, - macho->numSections = 4, + macho->macho_header = test_header, + macho->num_sections = 4, macho->sections = expected_sections; symbol_table[0] = symbol1; @@ -172,32 +172,32 @@ static void cleanup(void) { } } -static void test_read_macho_file(MachOFile *expected_macho) { - MachOFile test_macho_file; +static void test_read_macho_file(machofile *expected_macho) { + machofile test_macho_file; if(!read_macho_file(TEST_FILE, &test_macho_file)) { LOG_ERROR("Something in read_macho_file broke"); exit(EXIT_FAILURE); } - if (memcmp(&test_macho_file.machHeader, &expected_macho->machHeader, sizeof(MachOHeader)) != 0) { + if (memcmp(&test_macho_file.macho_header, &expected_macho->macho_header, sizeof(macho_header)) != 0) { LOG_ERROR("test_read_macho_file: read header is different than expected"); exit(EXIT_FAILURE); } - if (test_macho_file.numSections != expected_macho->numSections) { + if (test_macho_file.num_sections != expected_macho->num_sections) { LOG_ERROR("test_read_macho_file: read number of sections is dfferent than expected"); exit(EXIT_FAILURE); } - if (memcmp(test_macho_file.sections, expected_macho->sections, test_macho_file.numSections * sizeof(SectionInfo)) != 0) { + if (memcmp(test_macho_file.sections, expected_macho->sections, test_macho_file.num_sections * sizeof(section_info)) != 0) { LOG_ERROR("test_read_macho_file: read section information is different than expected"); printf("test:\n"); - print_hex(test_macho_file.sections, test_macho_file.numSections * sizeof(SectionInfo)); + print_hex(test_macho_file.sections, test_macho_file.num_sections * sizeof(section_info)); printf("expected:\n"); - print_hex(expected_macho->sections, expected_macho->numSections * sizeof(SectionInfo)); + print_hex(expected_macho->sections, expected_macho->num_sections * sizeof(section_info)); exit(EXIT_FAILURE); } } -static void test_get_macho_section_data(MachOFile *expected_macho, nList* expected_symtab, const char* expected_strtab) { +static void test_get_macho_section_data(machofile *expected_macho, symbol_info* expected_symtab, const char* expected_strtab) { uint8_t *text_section = NULL; size_t text_section_size; char expected_text_section[] = TEXT_DATA; @@ -238,7 +238,7 @@ static void test_get_macho_section_data(MachOFile *expected_macho, nList* expect } } -static void test_find_macho_symbol_index(MachOFile *expected_macho, const char *expected_strtab) { +static void test_find_macho_symbol_index(machofile *expected_macho, const char *expected_strtab) { uint8_t *symbol_table = NULL; size_t symbol_table_size; @@ -256,8 +256,8 @@ static void test_find_macho_symbol_index(MachOFile *expected_macho, const char * int main(int argc, char *argv[]) { - MachOFile *expected_macho = malloc(sizeof(MachOFile)); - nList *expected_symtab = malloc(NUM_SYMS * sizeof(nList)); + machofile *expected_macho = malloc(sizeof(machofile)); + symbol_info *expected_symtab = malloc(NUM_SYMS * sizeof(symbol_info)); char expected_strtab[] = "__text\0__const\0symbol1\0symbol2\0"; create_test_macho_file(expected_macho, expected_symtab, expected_strtab); From c25ce900e955e2938c91d5628ad8a845a4fa9338 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 25 Jan 2024 09:47:18 -0800 Subject: [PATCH 15/36] add better failure handling to macho parser --- .../inject_hash/macho_parser/macho_parser.c | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index f74d9c77e3..2f163e07b1 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -15,19 +15,25 @@ // Documentation for the Mach-O structs can be found in macho-o/loader.h and mach-o/nlist.h int read_macho_file(const char *filename, machofile *macho) { - FILE *file = fopen(filename, "rb"); + FILE *file = NULL; + load_cmd *load_commands = NULL; + int ret = 1; + + file = fopen(filename, "rb"); if (!file) { LOG_ERROR("Error opening file %s", filename); - return 0; + ret = 0; + goto end; } fread(&macho->macho_header, sizeof(macho_header), 1, file); if(macho->macho_header.magic != MH_MAGIC_64) { LOG_ERROR("File is not a 64-bit Mach-O file"); - return 0; + ret = 0; + goto end; } - load_cmd *load_commands = malloc(macho->macho_header.sizeofcmds); + load_commands = malloc(macho->macho_header.sizeofcmds); fread(load_commands, macho->macho_header.sizeofcmds, 1, file); // We're only looking for __text, __const in the __TEXT segment, and the string & symbol tables @@ -64,9 +70,14 @@ int read_macho_file(const char *filename, machofile *macho) { } - free(load_commands); - fclose(file); - return 1; +end: + if (load_commands != NULL) { + free(load_commands); + } + if (file != NULL) { + fclose(file); + } + return ret; } void free_macho_file(machofile *macho) { @@ -78,18 +89,21 @@ void free_macho_file(machofile *macho) { // Takes a filename, machofile struct, the name of the section to get data for, and pointers to size & offset as input // size and offset pointers are set to the size and offset of the section retrived in the file. uint8_t* get_macho_section_data(const char *filename, machofile *macho, const char *sectionName, size_t *size, uint32_t *offset) { - FILE *file = fopen(filename, "rb"); + FILE *file = NULL; + uint8_t *sectionData = NULL; + uint8_t *ret = NULL; + + file = fopen(filename, "rb"); if (!file) { LOG_ERROR("Error opening file %s", filename); - return NULL; + goto end; } for (uint32_t i = 0; i < macho->num_sections; i++) { if (strcmp(macho->sections[i].name, sectionName) == 0) { - uint8_t *sectionData = malloc(macho->sections[i].size); + sectionData = malloc(macho->sections[i].size); if (!sectionData) { - fclose(file); LOG_ERROR("Error allocating memory for section data"); - return NULL; + goto end; } fseek(file, macho->sections[i].offset, SEEK_SET); @@ -102,23 +116,32 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch *offset = macho->sections[i].offset; } - fclose(file); - return sectionData; + ret = sectionData; + goto end; } } LOG_ERROR("Section %s not found in macho file %s", sectionName, filename); - fclose(file); - return NULL; +end: + if (file != NULL) { + fclose(file); + } + if (ret == NULL && sectionData != NULL) { + free(sectionData); + } + return ret; } uint32_t find_macho_symbol_index(uint8_t *symbolTableData, size_t symbolTableSize, uint8_t *stringTableData, size_t stringTableSize, const char *symbolName, uint32_t *base) { + char* stringTable = NULL; + int ret = 0; + if (symbolTableData == NULL || stringTableData == NULL) { LOG_ERROR("Symbol and string table pointers cannot be null to find the symbol index"); - return 0; + goto end; } - char* stringTable = malloc(stringTableSize); + stringTable = malloc(stringTableSize); memcpy(stringTable, stringTableData, stringTableSize); int found = 0; @@ -131,19 +154,22 @@ uint32_t find_macho_symbol_index(uint8_t *symbolTableData, size_t symbolTableSiz found = 1; } else { LOG_ERROR("Duplicate symbol %s found", symbolName); - return 0; + goto end; } - } } - if (!found) { LOG_ERROR("Requested symbol %s not found", symbolName); + goto end; } - - free(stringTable); if (base) { index = index - *base; } - return index; + ret = index; + +end: + if (stringTable != NULL) { + free(stringTable); + } + return ret; } From 27bb119fae2e2036bdaa26badd1f91eec1ccfc8c Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 25 Jan 2024 09:51:30 -0800 Subject: [PATCH 16/36] remove more camel case --- .../inject_hash/macho_parser/macho_parser.c | 70 +++++++++---------- .../inject_hash/macho_parser/macho_parser.h | 4 +- .../macho_parser/tests/macho_tests.c | 6 ++ 3 files changed, 43 insertions(+), 37 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 2f163e07b1..f591e1a25c 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -41,7 +41,7 @@ int read_macho_file(const char *filename, machofile *macho) { macho->sections = malloc(macho->num_sections * sizeof(section_info)); // Iterate through load commands again to populate section information - uint32_t sectionIndex = 0; + uint32_t section_index = 0; for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { if (load_commands[i].cmd == LC_SEG) { segment_load_cmd *segment = (segment_load_cmd *)&load_commands[i]; @@ -49,23 +49,23 @@ int read_macho_file(const char *filename, machofile *macho) { section *sections = (section *)&segment[1]; for (uint32_t j = 0; j < segment->nsects; j++) { if (strcmp(sections[j].sectname, "__text") == 0 || strcmp(sections[j].sectname, "__const") == 0) { - macho->sections[sectionIndex].offset = sections[j].offset; - macho->sections[sectionIndex].size = sections[j].size; - strcpy(macho->sections[sectionIndex].name, sections[j].sectname); - sectionIndex++; + macho->sections[section_index].offset = sections[j].offset; + macho->sections[section_index].size = sections[j].size; + strcpy(macho->sections[section_index].name, sections[j].sectname); + section_index++; } } } } else if (load_commands[i].cmd == LC_SYMTAB) { symtab_load_cmd *symtab = (symtab_load_cmd *)&load_commands[i]; - macho->sections[sectionIndex].offset = symtab->symoff; - macho->sections[sectionIndex].size = symtab->nsyms * sizeof(symbol_info); - strcpy(macho->sections[sectionIndex].name, "__symbol_table"); - sectionIndex++; - macho->sections[sectionIndex].offset = symtab->stroff; - macho->sections[sectionIndex].size = symtab->strsize; - strcpy(macho->sections[sectionIndex].name, "__string_table"); - sectionIndex++; + macho->sections[section_index].offset = symtab->symoff; + macho->sections[section_index].size = symtab->nsyms * sizeof(symbol_info); + strcpy(macho->sections[section_index].name, "__symbol_table"); + section_index++; + macho->sections[section_index].offset = symtab->stroff; + macho->sections[section_index].size = symtab->strsize; + strcpy(macho->sections[section_index].name, "__string_table"); + section_index++; } } @@ -88,9 +88,9 @@ void free_macho_file(machofile *macho) { // Takes a filename, machofile struct, the name of the section to get data for, and pointers to size & offset as input // size and offset pointers are set to the size and offset of the section retrived in the file. -uint8_t* get_macho_section_data(const char *filename, machofile *macho, const char *sectionName, size_t *size, uint32_t *offset) { +uint8_t* get_macho_section_data(const char *filename, machofile *macho, const char *section_name, size_t *size, uint32_t *offset) { FILE *file = NULL; - uint8_t *sectionData = NULL; + uint8_t *section_data = NULL; uint8_t *ret = NULL; file = fopen(filename, "rb"); @@ -99,15 +99,15 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch goto end; } for (uint32_t i = 0; i < macho->num_sections; i++) { - if (strcmp(macho->sections[i].name, sectionName) == 0) { - sectionData = malloc(macho->sections[i].size); - if (!sectionData) { + if (strcmp(macho->sections[i].name, section_name) == 0) { + section_data = malloc(macho->sections[i].size); + if (!section_data) { LOG_ERROR("Error allocating memory for section data"); goto end; } fseek(file, macho->sections[i].offset, SEEK_SET); - fread(sectionData, 1, macho->sections[i].size, file); + fread(section_data, 1, macho->sections[i].size, file); if (size != NULL) { *size = macho->sections[i].size; @@ -116,50 +116,50 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch *offset = macho->sections[i].offset; } - ret = sectionData; + ret = section_data; goto end; } } - LOG_ERROR("Section %s not found in macho file %s", sectionName, filename); + LOG_ERROR("Section %s not found in macho file %s", section_name, filename); end: if (file != NULL) { fclose(file); } - if (ret == NULL && sectionData != NULL) { - free(sectionData); + if (ret == NULL && section_data != NULL) { + free(section_data); } return ret; } -uint32_t find_macho_symbol_index(uint8_t *symbolTableData, size_t symbolTableSize, uint8_t *stringTableData, size_t stringTableSize, const char *symbolName, uint32_t *base) { - char* stringTable = NULL; +uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table_size, uint8_t *string_table_data, size_t string_table_size, const char *symbol_name, uint32_t *base) { + char* string_table = NULL; int ret = 0; - if (symbolTableData == NULL || stringTableData == NULL) { + if (symbol_table_data == NULL || string_table_data == NULL) { LOG_ERROR("Symbol and string table pointers cannot be null to find the symbol index"); goto end; } - stringTable = malloc(stringTableSize); - memcpy(stringTable, stringTableData, stringTableSize); + string_table = malloc(string_table_size); + memcpy(string_table, string_table_data, string_table_size); int found = 0; uint32_t index = 0; - for (uint32_t i = 0; i < symbolTableSize / sizeof(symbol_info); i++) { - symbol_info *symbol = (symbol_info *)(symbolTableData + i * sizeof(symbol_info)); - if (strcmp(symbolName, &stringTable[symbol->n_un.n_strx]) == 0) { + for (uint32_t i = 0; i < symbol_table_size / sizeof(symbol_info); i++) { + symbol_info *symbol = (symbol_info *)(symbol_table_data + i * sizeof(symbol_info)); + if (strcmp(symbol_name, &string_table[symbol->n_un.n_strx]) == 0) { if (!found) { index = symbol->n_value; found = 1; } else { - LOG_ERROR("Duplicate symbol %s found", symbolName); + LOG_ERROR("Duplicate symbol %s found", symbol_name); goto end; } } } if (!found) { - LOG_ERROR("Requested symbol %s not found", symbolName); + LOG_ERROR("Requested symbol %s not found", symbol_name); goto end; } if (base) { @@ -168,8 +168,8 @@ uint32_t find_macho_symbol_index(uint8_t *symbolTableData, size_t symbolTableSiz ret = index; end: - if (stringTable != NULL) { - free(stringTable); + if (string_table != NULL) { + free(string_table); } return ret; } diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index 19b7ba77c6..b44b2197d0 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -33,7 +33,7 @@ typedef struct { int read_macho_file(const char *filename, machofile *macho); void free_macho_file(machofile *macho); void print_macho_section_info(machofile *macho); -uint8_t* get_macho_section_data(const char* filename, machofile *macho, const char *sectionName, size_t *size, uint32_t *offset); -uint32_t find_macho_symbol_index(uint8_t *sectionData, size_t sectionSize, uint8_t *stringTableData, size_t stringTableSize, const char *symbolName, uint32_t *base); +uint8_t* get_macho_section_data(const char* filename, machofile *macho, const char *section_name, size_t *size, uint32_t *offset); +uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table_size, uint8_t *string_table_data, size_t string_table_size, const char *symbol_name, uint32_t *base); #endif diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 940405007b..777fc292e9 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -254,6 +254,12 @@ static void test_find_macho_symbol_index(machofile *expected_macho, const char * printf("%u\n", symbol1_index); } +/** + * TODO: + * move all "global" variables into main function and pass into tests as arguments + * actually check symbol index instead of just printing it +*/ + int main(int argc, char *argv[]) { machofile *expected_macho = malloc(sizeof(machofile)); From 4ed681e3610ac73a53c3d05df0f24148a925c6ac Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 25 Jan 2024 11:00:23 -0800 Subject: [PATCH 17/36] move global variables and defines into main function --- .../macho_parser/tests/macho_tests.c | 65 ++++++++++++------- 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 777fc292e9..9fa5bc9121 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -8,11 +8,6 @@ #define TEST_FILE "test_macho" -#define TEXT_DATA {0xC3} -#define CONST_DATA "hi" - -#define NUM_SYMS 2 - static void print_hex(const void *ptr, size_t size) { for (size_t i = 0; i < size; i++) { printf("%02X", *((unsigned char *) ptr + i)); @@ -27,7 +22,7 @@ static void print_hex(const void *ptr, size_t size) { printf("\n"); } -static void create_test_macho_file(machofile *macho, symbol_info *symbol_table, const char* string_table) { +static void create_test_macho_file(int num_syms, int *text_data, char *const_data,machofile *macho, symbol_info *symbol_table, const char* string_table) { if (macho == NULL || symbol_table == NULL) { LOG_ERROR("macho and symbol_table must be allocated"); exit(EXIT_FAILURE); @@ -73,13 +68,13 @@ static void create_test_macho_file(machofile *macho, symbol_info *symbol_table, }; uint32_t symtab_command_symoff = const_section_offset + const_section_size; - uint32_t symtab_command_stroff = symtab_command_symoff + NUM_SYMS * sizeof(symbol_info); + uint32_t symtab_command_stroff = symtab_command_symoff + num_syms * sizeof(symbol_info); uint32_t symtab_command_strsize = 32; symtab_load_cmd test_symtab_command = { .cmd = LC_SYMTAB, .cmdsize = sizeof(symtab_load_cmd), .symoff = symtab_command_symoff, - .nsyms = NUM_SYMS, + .nsyms = num_syms, .stroff = symtab_command_stroff, .strsize = symtab_command_strsize, }; @@ -90,8 +85,8 @@ static void create_test_macho_file(machofile *macho, symbol_info *symbol_table, fwrite(&test_const_section, sizeof(section), 1, file); fwrite(&test_symtab_command, sizeof(symtab_load_cmd), 1, file); - char test_text_data[] = TEXT_DATA; - char test_const_data[] = CONST_DATA; + int *test_text_data = text_data; + char *test_const_data = const_data; fseek(file, test_text_section.offset, SEEK_SET); fwrite(test_text_data, sizeof(test_text_data), 1, file); @@ -141,7 +136,7 @@ static void create_test_macho_file(machofile *macho, symbol_info *symbol_table, section_info expected_symbol_table = { .name = "__symbol_table", - .size = NUM_SYMS * sizeof(symbol_info), + .size = num_syms * sizeof(symbol_info), .offset = symtab_command_symoff, }; @@ -197,14 +192,14 @@ static void test_read_macho_file(machofile *expected_macho) { } } -static void test_get_macho_section_data(machofile *expected_macho, symbol_info* expected_symtab, const char* expected_strtab) { +static void test_get_macho_section_data(int *text_data, char *const_data, machofile *expected_macho, symbol_info* expected_symtab, const char* expected_strtab) { uint8_t *text_section = NULL; size_t text_section_size; - char expected_text_section[] = TEXT_DATA; + int *expected_text_section = text_data; uint8_t *const_section = NULL; size_t const_section_size; - char expected_const_section[] = CONST_DATA; + char *expected_const_section = const_data; uint8_t *symbol_table = NULL; size_t symbol_table_size; @@ -236,9 +231,22 @@ static void test_get_macho_section_data(machofile *expected_macho, symbol_info* LOG_ERROR("string table is not equal to what was expected"); exit(EXIT_FAILURE); } + + if (text_section != NULL) { + free(text_section); + } + if (const_section != NULL) { + free(const_section); + } + if (symbol_table != NULL) { + free(symbol_table); + } + if (string_table != NULL) { + free(string_table); + } } -static void test_find_macho_symbol_index(machofile *expected_macho, const char *expected_strtab) { +static void test_find_macho_symbol_index(machofile *expected_macho, const char *expected_strtab, uint32_t expected_symbol_int) { uint8_t *symbol_table = NULL; size_t symbol_table_size; @@ -251,25 +259,38 @@ static void test_find_macho_symbol_index(machofile *expected_macho, const char * uint32_t symbol1_index; symbol1_index = find_macho_symbol_index(symbol_table, symbol_table_size, string_table, string_table_size, "symbol1", NULL); - printf("%u\n", symbol1_index); + if (symbol_table != NULL) { + free(symbol_table); + } + if (string_table != NULL) { + free(string_table); + } + + if (expected_symbol_int != symbol1_index) { + LOG_ERROR("found index %u is not equal to expected index %u", symbol1_index, expected_symbol_int); + exit(EXIT_FAILURE); + } } /** * TODO: * move all "global" variables into main function and pass into tests as arguments - * actually check symbol index instead of just printing it */ int main(int argc, char *argv[]) { + int num_syms = 2; + char expected_strtab[] = "__text\0__const\0symbol1\0symbol2\0"; + int text_data[] = { 0xC3 }; + char const_data[] = "hi"; machofile *expected_macho = malloc(sizeof(machofile)); - symbol_info *expected_symtab = malloc(NUM_SYMS * sizeof(symbol_info)); - char expected_strtab[] = "__text\0__const\0symbol1\0symbol2\0"; + symbol_info *expected_symtab = malloc(num_syms * sizeof(symbol_info)); + uint32_t expected_symbol_ind = 15; - create_test_macho_file(expected_macho, expected_symtab, expected_strtab); + create_test_macho_file(num_syms, text_data, const_data, expected_macho, expected_symtab, expected_strtab); test_read_macho_file(expected_macho); - test_get_macho_section_data(expected_macho, expected_symtab, expected_strtab); - test_find_macho_symbol_index(expected_macho, expected_strtab); + test_get_macho_section_data(text_data, const_data, expected_macho, expected_symtab, expected_strtab); + test_find_macho_symbol_index(expected_macho, expected_strtab, expected_symbol_ind); free_macho_file(expected_macho); free(expected_symtab); From 7bb4180214db302e540584420e16b105e4d8cb94 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 25 Jan 2024 11:00:31 -0800 Subject: [PATCH 18/36] remove todos --- util/fipstools/inject_hash/macho_parser/macho_parser.c | 7 ------- .../fipstools/inject_hash/macho_parser/tests/macho_tests.c | 5 ----- 2 files changed, 12 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index f591e1a25c..c8ef93a5fb 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -6,13 +6,6 @@ #include "common.h" #include "macho_parser.h" -/** - * TODOs - * use goto for cleaner exits in this and tests (if necessary) - * make all variable and function names snake case in all files - * finish tests -*/ - // Documentation for the Mach-O structs can be found in macho-o/loader.h and mach-o/nlist.h int read_macho_file(const char *filename, machofile *macho) { FILE *file = NULL; diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 9fa5bc9121..8b79677935 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -272,11 +272,6 @@ static void test_find_macho_symbol_index(machofile *expected_macho, const char * } } -/** - * TODO: - * move all "global" variables into main function and pass into tests as arguments -*/ - int main(int argc, char *argv[]) { int num_syms = 2; char expected_strtab[] = "__text\0__const\0symbol1\0symbol2\0"; From 093f28d1200999ab5a5d25fc4c7b70d8fbb32950 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Mon, 5 Feb 2024 16:19:23 -0800 Subject: [PATCH 19/36] start convert C tests to C++ tests utilizing google test --- .../inject_hash/macho_parser/macho_parser.c | 2 +- .../inject_hash/macho_parser/macho_parser.h | 9 +- .../macho_parser/tests/CMakeLists.txt | 9 +- .../macho_parser/tests/macho_tests.c | 14 +- .../macho_parser/tests/macho_tests.cc | 43 ++++ .../macho_parser/tests/macho_tests.h | 192 ++++++++++++++++++ 6 files changed, 259 insertions(+), 10 deletions(-) create mode 100644 util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc create mode 100644 util/fipstools/inject_hash/macho_parser/tests/macho_tests.h diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index c8ef93a5fb..a2341e8b6a 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -39,7 +39,7 @@ int read_macho_file(const char *filename, machofile *macho) { if (load_commands[i].cmd == LC_SEG) { segment_load_cmd *segment = (segment_load_cmd *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { - section *sections = (section *)&segment[1]; + section_data *sections = (section_data *)&segment[1]; for (uint32_t j = 0; j < segment->nsects; j++) { if (strcmp(sections[j].sectname, "__text") == 0 || strcmp(sections[j].sectname, "__const") == 0) { macho->sections[section_index].offset = sections[j].offset; diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index b44b2197d0..2bfc600571 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -3,6 +3,10 @@ #ifndef MACHO_PARSER_H #define MACHO_PARSER_H +#ifdef __cplusplus +extern "C" +{ +#endif #include #include @@ -21,7 +25,7 @@ typedef struct mach_header_64 macho_header; typedef struct load_command load_cmd; typedef struct segment_command_64 segment_load_cmd; typedef struct symtab_command symtab_load_cmd; -typedef struct section_64 section; +typedef struct section_64 section_data; typedef struct nlist_64 symbol_info; typedef struct { @@ -36,4 +40,7 @@ void print_macho_section_info(machofile *macho); uint8_t* get_macho_section_data(const char* filename, machofile *macho, const char *section_name, size_t *size, uint32_t *offset); uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table_size, uint8_t *string_table_data, size_t string_table_size, const char *symbol_name, uint32_t *base); +#ifdef __cplusplus +} // extern "C" +#endif #endif diff --git a/util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt b/util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt index 2c4b270c8d..59ffa32a58 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt +++ b/util/fipstools/inject_hash/macho_parser/tests/CMakeLists.txt @@ -2,7 +2,14 @@ if(FIPS AND APPLE) add_executable( test_macho_parser - macho_tests.c + macho_tests.cc ../macho_parser.c ) + + target_link_libraries( + test_macho_parser + + test_support_lib + boringssl_gtest_main + ) endif() diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c index 8b79677935..d770fd39f0 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c @@ -34,7 +34,7 @@ static void create_test_macho_file(int num_syms, int *text_data, char *const_dat exit(EXIT_FAILURE); } - uint32_t header_sizeofcmds = sizeof(segment_load_cmd) + 2 * sizeof(section) + sizeof(symtab_load_cmd); + uint32_t header_sizeofcmds = sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); uint32_t header_ncmds = 2; macho_header test_header = { .magic = MH_MAGIC_64, @@ -42,7 +42,7 @@ static void create_test_macho_file(int num_syms, int *text_data, char *const_dat .sizeofcmds = header_sizeofcmds, }; - uint32_t text_segment_cmdsize = sizeof(segment_load_cmd) + 2 * sizeof(section); + uint32_t text_segment_cmdsize = sizeof(segment_load_cmd) + 2 * sizeof(section_data); uint32_t text_segment_nsects = 2; segment_load_cmd test_text_segment = { .cmd = LC_SEGMENT_64, @@ -51,9 +51,9 @@ static void create_test_macho_file(int num_syms, int *text_data, char *const_dat .nsects = text_segment_nsects, }; - uint32_t text_section_offset = sizeof(macho_header) + sizeof(segment_load_cmd) + 2 * sizeof(section) + sizeof(symtab_load_cmd); + uint32_t text_section_offset = sizeof(macho_header) + sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); uint64_t text_section_size = 1; // {0xC3} - section test_text_section = { + section_data test_text_section = { .sectname = "__text", .size = text_section_size, .offset = text_section_offset, @@ -61,7 +61,7 @@ static void create_test_macho_file(int num_syms, int *text_data, char *const_dat uint32_t const_section_offset = text_section_offset + text_section_size; uint64_t const_section_size = 2; // "hi" - section test_const_section = { + section_data test_const_section = { .sectname = "__const", .size = const_section_size, .offset = const_section_offset, @@ -81,8 +81,8 @@ static void create_test_macho_file(int num_syms, int *text_data, char *const_dat fwrite(&test_header, sizeof(macho_header), 1, file); fwrite(&test_text_segment, sizeof(segment_load_cmd), 1, file); - fwrite(&test_text_section, sizeof(section), 1, file); - fwrite(&test_const_section, sizeof(section), 1, file); + fwrite(&test_text_section, sizeof(section_data), 1, file); + fwrite(&test_const_section, sizeof(section_data), 1, file); fwrite(&test_symtab_command, sizeof(symtab_load_cmd), 1, file); int *test_text_data = text_data; diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc new file mode 100644 index 0000000000..c58fb29f9b --- /dev/null +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc @@ -0,0 +1,43 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + +#include + +#include "../common.h" +#include "macho_tests.h" + +#define TEST_FILE "test_macho" + +// static void print_hex(const void *ptr, size_t size) { +// for (size_t i = 0; i < size; i++) { +// printf("%02X", *((unsigned char *) ptr + i)); +// if ((i+1)%4 == 0) { +// printf(" "); +// } + +// if((i+1)%32 == 0) { +// printf("\n"); +// } +// } +// printf("\n"); +// } + +machofile *MachoTestFixture::expected_macho; +symbol_info *MachoTestFixture::expected_symtab; +uint32_t MachoTestFixture::num_syms; +char MachoTestFixture::expected_strtab[31]; +int MachoTestFixture::text_data[1]; +char MachoTestFixture::const_data[2]; + +TEST_F(MachoTestFixture, TestReadMachOFile) { + machofile test_macho_file; + if(!read_macho_file(TEST_FILE, &test_macho_file)) { + LOG_ERROR("Failed to read macho_file"); + } + + EXPECT_TRUE(memcmp(&test_macho_file.macho_header, &expected_macho->macho_header, sizeof(macho_header)) == 0); + + EXPECT_EQ(test_macho_file.num_sections, expected_macho->num_sections); + + EXPECT_TRUE(memcmp(test_macho_file.sections, expected_macho->sections, test_macho_file.num_sections * sizeof(section_info)) == 0); +} diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h new file mode 100644 index 0000000000..8b13a47fc6 --- /dev/null +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -0,0 +1,192 @@ +#include + +#include "../macho_parser.h" + +#define TEST_FILE "test_macho" +#define EXPECTED_STRTAB_SIZE 31 +#define TEXT_DATA_SIZE 1 +#define CONST_DATA_SIZE 2 + +class MachoTestFixture : public ::testing::Test { +protected: + static machofile *expected_macho; + static symbol_info *expected_symtab; + static uint32_t num_syms; + static char expected_strtab[EXPECTED_STRTAB_SIZE]; + static int text_data[TEXT_DATA_SIZE]; + static char const_data[CONST_DATA_SIZE]; + + static void print_hex(const void *ptr, size_t size) { + for (size_t i = 0; i < size; i++) { + printf("%02X", *((unsigned char *) ptr + i)); + if ((i+1)%4 == 0) { + printf(" "); + } + + if((i+1)%32 == 0) { + printf("\n"); + } + } + printf("\n"); + } + + static void SetUpTestSuite() { + + num_syms = 2; + memcpy(expected_strtab, "__text\0__const\0symbol1\0symbol2\0", EXPECTED_STRTAB_SIZE); + text_data[0] = 0xC3; + memcpy(const_data, "hi", CONST_DATA_SIZE); + + FILE *file = fopen(TEST_FILE, "wb"); + if (!file) { + LOG_ERROR("Error with fopen() on %s file", TEST_FILE); + } + + uint32_t header_sizeofcmds = sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); + uint32_t header_ncmds = 2; + macho_header test_header = { + .magic = MH_MAGIC_64, + .ncmds = header_ncmds, + .sizeofcmds = header_sizeofcmds, + }; + + uint32_t text_segment_cmdsize = sizeof(segment_load_cmd) + 2 * sizeof(section_data); + uint32_t text_segment_nsects = 2; + segment_load_cmd test_text_segment = { + .cmd = LC_SEGMENT_64, + .cmdsize = text_segment_cmdsize, + .segname = "__TEXT", + .nsects = text_segment_nsects, + }; + + uint32_t text_section_offset = sizeof(macho_header) + sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); + uint64_t text_section_size = TEXT_DATA_SIZE; // {0xC3} + section_data test_text_section = { + .sectname = "__text", + .size = text_section_size, + .offset = text_section_offset, + }; + + uint32_t const_section_offset = text_section_offset + text_section_size; + uint64_t const_section_size = CONST_DATA_SIZE; // "hi" + section_data test_const_section = { + .sectname = "__const", + .size = const_section_size, + .offset = const_section_offset, + }; + + uint32_t symtab_command_symoff = const_section_offset + const_section_size; + uint32_t symtab_command_stroff = symtab_command_symoff + num_syms * sizeof(symbol_info); + uint32_t symtab_command_strsize = 32; + symtab_load_cmd test_symtab_command = { + .cmd = LC_SYMTAB, + .cmdsize = sizeof(symtab_load_cmd), + .symoff = symtab_command_symoff, + .nsyms = num_syms, + .stroff = symtab_command_stroff, + .strsize = symtab_command_strsize, + }; + + fwrite(&test_header, sizeof(macho_header), 1, file); + fwrite(&test_text_segment, sizeof(segment_load_cmd), 1, file); + fwrite(&test_text_section, sizeof(section_data), 1, file); + fwrite(&test_const_section, sizeof(section_data), 1, file); + fwrite(&test_symtab_command, sizeof(symtab_load_cmd), 1, file); + + int *test_text_data = text_data; + char *test_const_data = const_data; + + fseek(file, test_text_section.offset, SEEK_SET); + fwrite(test_text_data, sizeof(*test_text_data), 1, file); + + fseek(file, test_const_section.offset, SEEK_SET); + fwrite(test_const_data, sizeof(*test_const_data), 1, file); + + symbol_info symbol1 = { + .n_un = {.n_strx = 15}, // Index into the string table + .n_type = 0, + .n_sect = 1, + .n_desc = 0, + .n_value = 15, // Address of the symbol + }; + + symbol_info symbol2 = { + .n_un = {.n_strx = 23}, // Index into the string table + .n_type = 0, + .n_sect = 2, + .n_desc = 0, + .n_value = 23, // Address of the symbol + }; + + fseek(file, symtab_command_symoff, SEEK_SET); + fwrite(&symbol1, sizeof(symbol_info), 1, file); + fwrite(&symbol2, sizeof(symbol_info), 1, file); + + // Write the string table + fseek(file, symtab_command_stroff, SEEK_SET); + fwrite(expected_strtab, symtab_command_strsize, 1, file); + + if (fclose(file) != 0) { + LOG_ERROR("Error closing file\n"); + } + + section_info *expected_text_section = (section_info*) malloc(sizeof(section_info)); + strcpy(expected_text_section->name, "__text"); + expected_text_section->size = text_section_size; + expected_text_section->offset = text_section_offset; + + section_info *expected_const_section = (section_info*) malloc(sizeof(section_info)); + strcpy(expected_const_section->name, "__const"); + expected_const_section->size = const_section_size; + expected_const_section->offset = const_section_offset; + + section_info *expected_symbol_table = (section_info*) malloc(sizeof(section_info)); + strcpy(expected_symbol_table->name, "__symbol_table"); + expected_symbol_table->size = num_syms * sizeof(symbol_info); + expected_symbol_table->offset = symtab_command_symoff; + + section_info *expected_string_table = (section_info*) malloc(sizeof(section_info)); + strcpy(expected_string_table->name, "__string_table"); + expected_string_table->size = symtab_command_strsize; + expected_string_table->offset = symtab_command_stroff; + + section_info *expected_sections = (section_info*) malloc(sizeof(section_info) * 4); + memcpy(&expected_sections[0], expected_text_section, sizeof(section_info)); + memcpy(&expected_sections[1], expected_const_section, sizeof(section_info)); + memcpy(&expected_sections[2], expected_symbol_table, sizeof(section_info)); + memcpy(&expected_sections[3], expected_string_table, sizeof(section_info)); + + free(expected_text_section); + free(expected_const_section); + free(expected_symbol_table); + free(expected_string_table); + + expected_macho = (machofile*) malloc(sizeof(machofile)); + expected_macho->macho_header = test_header; + expected_macho->num_sections = 4; + expected_macho->sections = expected_sections; + + expected_symtab = (symbol_info*) malloc(num_syms * sizeof(symbol_info)); + expected_symtab[0] = symbol1; + expected_symtab[1] = symbol2; + } + + static void TearDownTestSuite() { + if (expected_macho != NULL) { + free_macho_file(expected_macho); + } + if (expected_symtab != NULL) { + free(expected_symtab); + } + if (remove(TEST_FILE) != 0) { + LOG_ERROR("Error deleting %s", TEST_FILE); + } + } + + // void SetUp() override { + + // } + // void TearDown() override { + + // } +}; From d629c08e49e19ec958f232147dfa9d806876125e Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Tue, 6 Feb 2024 10:17:58 -0800 Subject: [PATCH 20/36] transfer remaining tests and remove legacy test file --- .../macho_parser/tests/macho_tests.c | 296 ------------------ .../macho_parser/tests/macho_tests.cc | 51 ++- .../macho_parser/tests/macho_tests.h | 24 +- 3 files changed, 55 insertions(+), 316 deletions(-) delete mode 100644 util/fipstools/inject_hash/macho_parser/tests/macho_tests.c diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c deleted file mode 100644 index d770fd39f0..0000000000 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.c +++ /dev/null @@ -1,296 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 OR ISC - -#include - -#include "../common.h" -#include "../macho_parser.h" - -#define TEST_FILE "test_macho" - -static void print_hex(const void *ptr, size_t size) { - for (size_t i = 0; i < size; i++) { - printf("%02X", *((unsigned char *) ptr + i)); - if ((i+1)%4 == 0) { - printf(" "); - } - - if((i+1)%32 == 0) { - printf("\n"); - } - } - printf("\n"); -} - -static void create_test_macho_file(int num_syms, int *text_data, char *const_data,machofile *macho, symbol_info *symbol_table, const char* string_table) { - if (macho == NULL || symbol_table == NULL) { - LOG_ERROR("macho and symbol_table must be allocated"); - exit(EXIT_FAILURE); - } - - FILE *file = fopen(TEST_FILE, "wb"); - if (!file) { - LOG_ERROR("Error with fopen() on %s file", TEST_FILE); - exit(EXIT_FAILURE); - } - - uint32_t header_sizeofcmds = sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); - uint32_t header_ncmds = 2; - macho_header test_header = { - .magic = MH_MAGIC_64, - .ncmds = header_ncmds, - .sizeofcmds = header_sizeofcmds, - }; - - uint32_t text_segment_cmdsize = sizeof(segment_load_cmd) + 2 * sizeof(section_data); - uint32_t text_segment_nsects = 2; - segment_load_cmd test_text_segment = { - .cmd = LC_SEGMENT_64, - .cmdsize = text_segment_cmdsize, - .segname = "__TEXT", - .nsects = text_segment_nsects, - }; - - uint32_t text_section_offset = sizeof(macho_header) + sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); - uint64_t text_section_size = 1; // {0xC3} - section_data test_text_section = { - .sectname = "__text", - .size = text_section_size, - .offset = text_section_offset, - }; - - uint32_t const_section_offset = text_section_offset + text_section_size; - uint64_t const_section_size = 2; // "hi" - section_data test_const_section = { - .sectname = "__const", - .size = const_section_size, - .offset = const_section_offset, - }; - - uint32_t symtab_command_symoff = const_section_offset + const_section_size; - uint32_t symtab_command_stroff = symtab_command_symoff + num_syms * sizeof(symbol_info); - uint32_t symtab_command_strsize = 32; - symtab_load_cmd test_symtab_command = { - .cmd = LC_SYMTAB, - .cmdsize = sizeof(symtab_load_cmd), - .symoff = symtab_command_symoff, - .nsyms = num_syms, - .stroff = symtab_command_stroff, - .strsize = symtab_command_strsize, - }; - - fwrite(&test_header, sizeof(macho_header), 1, file); - fwrite(&test_text_segment, sizeof(segment_load_cmd), 1, file); - fwrite(&test_text_section, sizeof(section_data), 1, file); - fwrite(&test_const_section, sizeof(section_data), 1, file); - fwrite(&test_symtab_command, sizeof(symtab_load_cmd), 1, file); - - int *test_text_data = text_data; - char *test_const_data = const_data; - - fseek(file, test_text_section.offset, SEEK_SET); - fwrite(test_text_data, sizeof(test_text_data), 1, file); - - fseek(file, test_const_section.offset, SEEK_SET); - fwrite(test_const_data, sizeof(test_const_data), 1, file); - - symbol_info symbol1 = { - .n_un = {.n_strx = 15}, // Index into the string table - .n_type = 0, - .n_sect = 1, - .n_desc = 0, - .n_value = 15, // Address of the symbol - }; - - symbol_info symbol2 = { - .n_un = {.n_strx = 23}, // Index into the string table - .n_type = 0, - .n_sect = 2, - .n_desc = 0, - .n_value = 23, // Address of the symbol - }; - - fseek(file, symtab_command_symoff, SEEK_SET); - fwrite(&symbol1, sizeof(symbol_info), 1, file); - fwrite(&symbol2, sizeof(symbol_info), 1, file); - - // Write the string table - fseek(file, symtab_command_stroff, SEEK_SET); - fwrite(string_table, symtab_command_strsize, 1, file); - - if (fclose(file) != 0) { - LOG_ERROR("Error closing file\n"); - } - - section_info expected_text_section = { - .name = "__text", - .size = text_section_size, - .offset = text_section_offset, - }; - - section_info expected_const_section = { - .name = "__const", - .size = const_section_size, - .offset = const_section_offset, - }; - - section_info expected_symbol_table = { - .name = "__symbol_table", - .size = num_syms * sizeof(symbol_info), - .offset = symtab_command_symoff, - }; - - section_info expected_string_table = { - .name = "__string_table", - .size = symtab_command_strsize, - .offset = symtab_command_stroff, - }; - - section_info *expected_sections = malloc(sizeof(section_info) * 4); - expected_sections[0] = expected_text_section; - expected_sections[1] = expected_const_section; - expected_sections[2] = expected_symbol_table; - expected_sections[3] = expected_string_table; - - macho->macho_header = test_header, - macho->num_sections = 4, - macho->sections = expected_sections; - - symbol_table[0] = symbol1; - symbol_table[1] = symbol2; -} - -static void cleanup(void) { - if (remove(TEST_FILE) != 0) { - LOG_ERROR("Error deleting %s", TEST_FILE); - exit(EXIT_FAILURE); - } -} - -static void test_read_macho_file(machofile *expected_macho) { - machofile test_macho_file; - if(!read_macho_file(TEST_FILE, &test_macho_file)) { - LOG_ERROR("Something in read_macho_file broke"); - exit(EXIT_FAILURE); - } - - if (memcmp(&test_macho_file.macho_header, &expected_macho->macho_header, sizeof(macho_header)) != 0) { - LOG_ERROR("test_read_macho_file: read header is different than expected"); - exit(EXIT_FAILURE); - } - if (test_macho_file.num_sections != expected_macho->num_sections) { - LOG_ERROR("test_read_macho_file: read number of sections is dfferent than expected"); - exit(EXIT_FAILURE); - } - if (memcmp(test_macho_file.sections, expected_macho->sections, test_macho_file.num_sections * sizeof(section_info)) != 0) { - LOG_ERROR("test_read_macho_file: read section information is different than expected"); - printf("test:\n"); - print_hex(test_macho_file.sections, test_macho_file.num_sections * sizeof(section_info)); - printf("expected:\n"); - print_hex(expected_macho->sections, expected_macho->num_sections * sizeof(section_info)); - exit(EXIT_FAILURE); - } -} - -static void test_get_macho_section_data(int *text_data, char *const_data, machofile *expected_macho, symbol_info* expected_symtab, const char* expected_strtab) { - uint8_t *text_section = NULL; - size_t text_section_size; - int *expected_text_section = text_data; - - uint8_t *const_section = NULL; - size_t const_section_size; - char *expected_const_section = const_data; - - uint8_t *symbol_table = NULL; - size_t symbol_table_size; - - uint8_t *string_table = NULL; - size_t string_table_size; - - text_section = get_macho_section_data(TEST_FILE, expected_macho, "__text", &text_section_size, NULL); - if (memcmp(text_section, expected_text_section, text_section_size) != 0) { - LOG_ERROR("text section is not equal to what was expected"); - exit(EXIT_FAILURE); - } - - const_section = get_macho_section_data(TEST_FILE, expected_macho, "__const", &const_section_size, NULL); - if (memcmp(const_section, expected_const_section, const_section_size) != 0) { - LOG_ERROR("const section is not equal to what was expected"); - exit(EXIT_FAILURE); - } - - // Something about calling this function on the symbol table causes something to segfault occasionally - symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); - if (memcmp(symbol_table, expected_symtab, symbol_table_size) != 0) { - LOG_ERROR("symbol table is not equal to what was expected"); - exit(EXIT_FAILURE); - } - - string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); - if (memcmp(string_table, expected_strtab, string_table_size) != 0) { - LOG_ERROR("string table is not equal to what was expected"); - exit(EXIT_FAILURE); - } - - if (text_section != NULL) { - free(text_section); - } - if (const_section != NULL) { - free(const_section); - } - if (symbol_table != NULL) { - free(symbol_table); - } - if (string_table != NULL) { - free(string_table); - } -} - -static void test_find_macho_symbol_index(machofile *expected_macho, const char *expected_strtab, uint32_t expected_symbol_int) { - uint8_t *symbol_table = NULL; - size_t symbol_table_size; - - uint8_t *string_table = NULL; - size_t string_table_size; - - symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); - string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); - - uint32_t symbol1_index; - symbol1_index = find_macho_symbol_index(symbol_table, symbol_table_size, string_table, string_table_size, "symbol1", NULL); - - if (symbol_table != NULL) { - free(symbol_table); - } - if (string_table != NULL) { - free(string_table); - } - - if (expected_symbol_int != symbol1_index) { - LOG_ERROR("found index %u is not equal to expected index %u", symbol1_index, expected_symbol_int); - exit(EXIT_FAILURE); - } -} - -int main(int argc, char *argv[]) { - int num_syms = 2; - char expected_strtab[] = "__text\0__const\0symbol1\0symbol2\0"; - int text_data[] = { 0xC3 }; - char const_data[] = "hi"; - - machofile *expected_macho = malloc(sizeof(machofile)); - symbol_info *expected_symtab = malloc(num_syms * sizeof(symbol_info)); - uint32_t expected_symbol_ind = 15; - - create_test_macho_file(num_syms, text_data, const_data, expected_macho, expected_symtab, expected_strtab); - test_read_macho_file(expected_macho); - test_get_macho_section_data(text_data, const_data, expected_macho, expected_symtab, expected_strtab); - test_find_macho_symbol_index(expected_macho, expected_strtab, expected_symbol_ind); - - free_macho_file(expected_macho); - free(expected_symtab); - cleanup(); - - printf("All tests passed\n"); - return 0; -} diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc index c58fb29f9b..6e15bd4e67 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc @@ -25,19 +25,58 @@ machofile *MachoTestFixture::expected_macho; symbol_info *MachoTestFixture::expected_symtab; uint32_t MachoTestFixture::num_syms; -char MachoTestFixture::expected_strtab[31]; -int MachoTestFixture::text_data[1]; -char MachoTestFixture::const_data[2]; +char MachoTestFixture::expected_strtab[EXPECTED_STRTAB_SIZE]; +int MachoTestFixture::text_data[TEXT_DATA_SIZE]; +char MachoTestFixture::const_data[CONST_DATA_SIZE]; +uint32_t MachoTestFixture::expected_symbol1_ind; +uint32_t MachoTestFixture::expected_symbol2_ind; -TEST_F(MachoTestFixture, TestReadMachOFile) { +TEST_F(MachoTestFixture, TestReadMachoFile) { machofile test_macho_file; if(!read_macho_file(TEST_FILE, &test_macho_file)) { LOG_ERROR("Failed to read macho_file"); } EXPECT_TRUE(memcmp(&test_macho_file.macho_header, &expected_macho->macho_header, sizeof(macho_header)) == 0); - EXPECT_EQ(test_macho_file.num_sections, expected_macho->num_sections); - EXPECT_TRUE(memcmp(test_macho_file.sections, expected_macho->sections, test_macho_file.num_sections * sizeof(section_info)) == 0); } + +TEST_F(MachoTestFixture, TestGetMachoSectionData) { + uint8_t *text_section = NULL; + size_t text_section_size; + + uint8_t *const_section = NULL; + size_t const_section_size; + + uint8_t *symbol_table = NULL; + size_t symbol_table_size; + + uint8_t *string_table = NULL; + size_t string_table_size; + + text_section = get_macho_section_data(TEST_FILE, expected_macho, "__text", &text_section_size, NULL); + const_section = get_macho_section_data(TEST_FILE, expected_macho, "__const", &const_section_size, NULL); + symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); + string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); + + ASSERT_TRUE(memcmp(text_section, text_data, text_section_size) == 0); + ASSERT_TRUE(memcmp(const_section, const_data, const_section_size) == 0); + ASSERT_TRUE(memcmp(symbol_table, expected_symtab, symbol_table_size) == 0); + ASSERT_TRUE(memcmp(string_table, expected_strtab, string_table_size) == 0); +} + +TEST_F(MachoTestFixture, TestFindMachoSymbolIndex) { + uint8_t *symbol_table = NULL; + size_t symbol_table_size; + + uint8_t *string_table = NULL; + size_t string_table_size; + + symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); + string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); + + uint32_t symbol1_index = find_macho_symbol_index(symbol_table, symbol_table_size, string_table, string_table_size, "symbol1", NULL); + + ASSERT_EQ(symbol1_index, expected_symbol1_ind); +} diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index 8b13a47fc6..42f3e594bb 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -15,6 +15,8 @@ class MachoTestFixture : public ::testing::Test { static char expected_strtab[EXPECTED_STRTAB_SIZE]; static int text_data[TEXT_DATA_SIZE]; static char const_data[CONST_DATA_SIZE]; + static uint32_t expected_symbol1_ind; + static uint32_t expected_symbol2_ind; static void print_hex(const void *ptr, size_t size) { for (size_t i = 0; i < size; i++) { @@ -31,11 +33,12 @@ class MachoTestFixture : public ::testing::Test { } static void SetUpTestSuite() { - num_syms = 2; memcpy(expected_strtab, "__text\0__const\0symbol1\0symbol2\0", EXPECTED_STRTAB_SIZE); text_data[0] = 0xC3; memcpy(const_data, "hi", CONST_DATA_SIZE); + expected_symbol1_ind = 15; + expected_symbol2_ind = 23; FILE *file = fopen(TEST_FILE, "wb"); if (!file) { @@ -97,25 +100,25 @@ class MachoTestFixture : public ::testing::Test { char *test_const_data = const_data; fseek(file, test_text_section.offset, SEEK_SET); - fwrite(test_text_data, sizeof(*test_text_data), 1, file); + fwrite(test_text_data, sizeof(test_text_data), 1, file); fseek(file, test_const_section.offset, SEEK_SET); - fwrite(test_const_data, sizeof(*test_const_data), 1, file); + fwrite(test_const_data, sizeof(test_const_data), 1, file); symbol_info symbol1 = { - .n_un = {.n_strx = 15}, // Index into the string table + .n_un = {.n_strx = expected_symbol1_ind}, // Index into the string table .n_type = 0, .n_sect = 1, .n_desc = 0, - .n_value = 15, // Address of the symbol + .n_value = expected_symbol1_ind, // Address of the symbol }; symbol_info symbol2 = { - .n_un = {.n_strx = 23}, // Index into the string table + .n_un = {.n_strx = expected_symbol2_ind}, // Index into the string table .n_type = 0, .n_sect = 2, .n_desc = 0, - .n_value = 23, // Address of the symbol + .n_value = expected_symbol2_ind, // Address of the symbol }; fseek(file, symtab_command_symoff, SEEK_SET); @@ -182,11 +185,4 @@ class MachoTestFixture : public ::testing::Test { LOG_ERROR("Error deleting %s", TEST_FILE); } } - - // void SetUp() override { - - // } - // void TearDown() override { - - // } }; From 24dd3d0a72c8912b77d5f4e03906afcf8ddb4c16 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Tue, 6 Feb 2024 10:25:05 -0800 Subject: [PATCH 21/36] clean up code --- .../inject_hash/macho_parser/macho_parser.c | 3 --- .../macho_parser/tests/macho_tests.cc | 14 ------------ .../macho_parser/tests/macho_tests.h | 22 ++++--------------- 3 files changed, 4 insertions(+), 35 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index a2341e8b6a..0ab541a6a5 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -33,7 +33,6 @@ int read_macho_file(const char *filename, machofile *macho) { macho->num_sections = 4; macho->sections = malloc(macho->num_sections * sizeof(section_info)); - // Iterate through load commands again to populate section information uint32_t section_index = 0; for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { if (load_commands[i].cmd == LC_SEG) { @@ -79,8 +78,6 @@ void free_macho_file(machofile *macho) { macho = NULL; } -// Takes a filename, machofile struct, the name of the section to get data for, and pointers to size & offset as input -// size and offset pointers are set to the size and offset of the section retrived in the file. uint8_t* get_macho_section_data(const char *filename, machofile *macho, const char *section_name, size_t *size, uint32_t *offset) { FILE *file = NULL; uint8_t *section_data = NULL; diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc index 6e15bd4e67..1e32c6dee0 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc @@ -8,20 +8,6 @@ #define TEST_FILE "test_macho" -// static void print_hex(const void *ptr, size_t size) { -// for (size_t i = 0; i < size; i++) { -// printf("%02X", *((unsigned char *) ptr + i)); -// if ((i+1)%4 == 0) { -// printf(" "); -// } - -// if((i+1)%32 == 0) { -// printf("\n"); -// } -// } -// printf("\n"); -// } - machofile *MachoTestFixture::expected_macho; symbol_info *MachoTestFixture::expected_symtab; uint32_t MachoTestFixture::num_syms; diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index 42f3e594bb..784004c08f 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -18,20 +18,6 @@ class MachoTestFixture : public ::testing::Test { static uint32_t expected_symbol1_ind; static uint32_t expected_symbol2_ind; - static void print_hex(const void *ptr, size_t size) { - for (size_t i = 0; i < size; i++) { - printf("%02X", *((unsigned char *) ptr + i)); - if ((i+1)%4 == 0) { - printf(" "); - } - - if((i+1)%32 == 0) { - printf("\n"); - } - } - printf("\n"); - } - static void SetUpTestSuite() { num_syms = 2; memcpy(expected_strtab, "__text\0__const\0symbol1\0symbol2\0", EXPECTED_STRTAB_SIZE); @@ -106,19 +92,19 @@ class MachoTestFixture : public ::testing::Test { fwrite(test_const_data, sizeof(test_const_data), 1, file); symbol_info symbol1 = { - .n_un = {.n_strx = expected_symbol1_ind}, // Index into the string table + .n_un = {.n_strx = expected_symbol1_ind}, .n_type = 0, .n_sect = 1, .n_desc = 0, - .n_value = expected_symbol1_ind, // Address of the symbol + .n_value = expected_symbol1_ind, }; symbol_info symbol2 = { - .n_un = {.n_strx = expected_symbol2_ind}, // Index into the string table + .n_un = {.n_strx = expected_symbol2_ind}, .n_type = 0, .n_sect = 2, .n_desc = 0, - .n_value = expected_symbol2_ind, // Address of the symbol + .n_value = expected_symbol2_ind, }; fseek(file, symtab_command_symoff, SEEK_SET); From 42f007c3ba1d2aa2405519edec14bfafa2937bbc Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Tue, 6 Feb 2024 10:31:44 -0800 Subject: [PATCH 22/36] add missing copyright --- util/fipstools/inject_hash/macho_parser/tests/macho_tests.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index 784004c08f..d4c0056281 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -1,3 +1,6 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + #include #include "../macho_parser.h" From 0adaeda00c65603f03976911b1d1149c03519d40 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Tue, 6 Feb 2024 11:49:53 -0800 Subject: [PATCH 23/36] add macho parser tests to run_tests target --- CMakeLists.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5d5e868527..3cd50d256e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1027,6 +1027,16 @@ if(BUILD_TESTING) add_custom_target(fips_specific_tests_if_any) endif() + # Add macho parser tests if FIPS and on MacOS + if(FIPS AND APPLE) + add_custom_target( + macho_parser_tests + COMMAND ./util/fipstools/inject_hash/macho_parser/tests/test_macho_parser + WORKING_DIRECTORY ${PROJECT_BINARY_DIR} + ) + add_dependencies(fips_specific_tests_if_any macho_parser_tests) + endif() + # Read util/go_tests.txt into a CMake variable. file(READ util/go_tests.txt GO_TESTS) foreach(fips_specific_test ${GO_FIPS_TESTS}) From 622c5936f4686ec5597525ad3099181efd3362e1 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Fri, 9 Feb 2024 14:07:38 -0800 Subject: [PATCH 24/36] use calloc ensure correct contents of memcmp data --- .../inject_hash/macho_parser/tests/macho_tests.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index d4c0056281..f7362b6544 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -122,22 +122,24 @@ class MachoTestFixture : public ::testing::Test { LOG_ERROR("Error closing file\n"); } - section_info *expected_text_section = (section_info*) malloc(sizeof(section_info)); + // We use calloc for the below four calls to ensure that the untouched parts are zeroized, + // as we will later memcmp the data to what we've read from the file. + section_info *expected_text_section = (section_info*) calloc(1, sizeof(section_info)); strcpy(expected_text_section->name, "__text"); expected_text_section->size = text_section_size; expected_text_section->offset = text_section_offset; - section_info *expected_const_section = (section_info*) malloc(sizeof(section_info)); + section_info *expected_const_section = (section_info*) calloc(1, sizeof(section_info)); strcpy(expected_const_section->name, "__const"); expected_const_section->size = const_section_size; expected_const_section->offset = const_section_offset; - section_info *expected_symbol_table = (section_info*) malloc(sizeof(section_info)); + section_info *expected_symbol_table = (section_info*) calloc(1, sizeof(section_info)); strcpy(expected_symbol_table->name, "__symbol_table"); expected_symbol_table->size = num_syms * sizeof(symbol_info); expected_symbol_table->offset = symtab_command_symoff; - section_info *expected_string_table = (section_info*) malloc(sizeof(section_info)); + section_info *expected_string_table = (section_info*) calloc(1, sizeof(section_info)); strcpy(expected_string_table->name, "__string_table"); expected_string_table->size = symtab_command_strsize; expected_string_table->offset = symtab_command_stroff; From 70b639a7b4510cc4a78e6689de84d05c3ecc1052 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Wed, 21 Feb 2024 13:37:33 -0800 Subject: [PATCH 25/36] address PR comments --- CMakeLists.txt | 1 + .../inject_hash/macho_parser/macho_parser.c | 65 +++++++++++------ .../inject_hash/macho_parser/macho_parser.h | 13 +++- .../macho_parser/tests/macho_tests.cc | 2 +- .../macho_parser/tests/macho_tests.h | 72 ++++++++++++++----- 5 files changed, 110 insertions(+), 43 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3cd50d256e..eed22f14e3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1033,6 +1033,7 @@ if(BUILD_TESTING) macho_parser_tests COMMAND ./util/fipstools/inject_hash/macho_parser/tests/test_macho_parser WORKING_DIRECTORY ${PROJECT_BINARY_DIR} + DEPENDS test_macho_parser ) add_dependencies(fips_specific_tests_if_any macho_parser_tests) endif() diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 0ab541a6a5..e9e927d0ee 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -10,32 +10,46 @@ int read_macho_file(const char *filename, machofile *macho) { FILE *file = NULL; load_cmd *load_commands = NULL; - int ret = 1; + uint32_t bytes_read; + int ret = 0; file = fopen(filename, "rb"); - if (!file) { + if (file == NULL) { LOG_ERROR("Error opening file %s", filename); - ret = 0; goto end; } - fread(&macho->macho_header, sizeof(macho_header), 1, file); - if(macho->macho_header.magic != MH_MAGIC_64) { + bytes_read = fread(&macho->macho_header, 1, sizeof(macho_header), file); + if (bytes_read != sizeof(macho_header)) { + LOG_ERROR("Error reading macho_header from file %s", filename); + goto end; + } + if (macho->macho_header.magic != MH_MAGIC_64) { LOG_ERROR("File is not a 64-bit Mach-O file"); - ret = 0; goto end; } load_commands = malloc(macho->macho_header.sizeofcmds); - fread(load_commands, macho->macho_header.sizeofcmds, 1, file); + if (load_commands == NULL) { + LOG_ERROR("Error allocating memory for load_commands"); + goto end; + } + bytes_read = fread(load_commands, 1, macho->macho_header.sizeofcmds, file); + if (bytes_read != macho->macho_header.sizeofcmds) { + LOG_ERROR("Error reading load commands from file %s", filename); + goto end; + } // We're only looking for __text, __const in the __TEXT segment, and the string & symbol tables macho->num_sections = 4; macho->sections = malloc(macho->num_sections * sizeof(section_info)); + if (macho->sections == NULL) { + LOG_ERROR("Error allocating memory for macho sections"); + } uint32_t section_index = 0; for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { - if (load_commands[i].cmd == LC_SEG) { + if (load_commands[i].cmd == LC_SEGMENT_64) { segment_load_cmd *segment = (segment_load_cmd *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { section_data *sections = (section_data *)&segment[1]; @@ -60,12 +74,10 @@ int read_macho_file(const char *filename, machofile *macho) { section_index++; } } - + ret = 1; end: - if (load_commands != NULL) { - free(load_commands); - } + free(load_commands); if (file != NULL) { fclose(file); } @@ -82,27 +94,32 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch FILE *file = NULL; uint8_t *section_data = NULL; uint8_t *ret = NULL; + uint32_t bytes_read; file = fopen(filename, "rb"); - if (!file) { + if (file == NULL) { LOG_ERROR("Error opening file %s", filename); goto end; } for (uint32_t i = 0; i < macho->num_sections; i++) { if (strcmp(macho->sections[i].name, section_name) == 0) { section_data = malloc(macho->sections[i].size); - if (!section_data) { + if (section_data == NULL) { LOG_ERROR("Error allocating memory for section data"); goto end; } fseek(file, macho->sections[i].offset, SEEK_SET); - fread(section_data, 1, macho->sections[i].size, file); + bytes_read = fread(section_data, 1, macho->sections[i].size, file); + if (bytes_read != macho->sections[i].size) { + LOG_ERROR("Error reading section data from file %s", filename); + goto end; + } if (size != NULL) { *size = macho->sections[i].size; } - if (offset) { + if (offset != NULL) { *offset = macho->sections[i].offset; } @@ -116,7 +133,7 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch if (file != NULL) { fclose(file); } - if (ret == NULL && section_data != NULL) { + if (ret == NULL) { free(section_data); } return ret; @@ -132,6 +149,10 @@ uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table } string_table = malloc(string_table_size); + if (string_table == NULL) { + LOG_ERROR("Error allocating memory for string table"); + goto end; + } memcpy(string_table, string_table_data, string_table_size); int found = 0; @@ -139,7 +160,7 @@ uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table for (uint32_t i = 0; i < symbol_table_size / sizeof(symbol_info); i++) { symbol_info *symbol = (symbol_info *)(symbol_table_data + i * sizeof(symbol_info)); if (strcmp(symbol_name, &string_table[symbol->n_un.n_strx]) == 0) { - if (!found) { + if (found == 0) { index = symbol->n_value; found = 1; } else { @@ -148,18 +169,16 @@ uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table } } } - if (!found) { + if (found == 0) { LOG_ERROR("Requested symbol %s not found", symbol_name); goto end; } - if (base) { + if (base != NULL) { index = index - *base; } ret = index; end: - if (string_table != NULL) { - free(string_table); - } + free(string_table); return ret; } diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index 2bfc600571..8068cc4b4a 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -18,7 +18,6 @@ typedef struct { } section_info; // Since we only support 64-bit architectures on Apple, we don't need to account for any of the 32-bit structures -#define LC_SEG LC_SEGMENT_64 #define BIT_MODIFIER 8 typedef struct mach_header_64 macho_header; @@ -34,10 +33,20 @@ typedef struct { uint32_t num_sections; } machofile; +// read_macho_file reads a Mach-O file [in] and populates a machofile struct [out] with its contents. +// It returns 0 on failure, 1 on success. int read_macho_file(const char *filename, machofile *macho); + +// free_macho_file frees the memory allocated to a machofile struct [in] void free_macho_file(machofile *macho); -void print_macho_section_info(machofile *macho); + +// get_macho_section_data retrieves data from a specific section [in] the provided Mach-O file [in]. +// In addition to returning a pointer to the retrieved data, or NULL if it doesn't find said section, +// it also populates the size [out] & offset [out] pointers provided they are not NULL. uint8_t* get_macho_section_data(const char* filename, machofile *macho, const char *section_name, size_t *size, uint32_t *offset); + +// find_macho_symbol_index finds the index of a symbol [in] in the Mach-O file's [in] symbol table. +// It returns the index on success, and 0 on failure. uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table_size, uint8_t *string_table_data, size_t string_table_size, const char *symbol_name, uint32_t *base); #ifdef __cplusplus diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc index 1e32c6dee0..2017b997ad 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc @@ -19,7 +19,7 @@ uint32_t MachoTestFixture::expected_symbol2_ind; TEST_F(MachoTestFixture, TestReadMachoFile) { machofile test_macho_file; - if(!read_macho_file(TEST_FILE, &test_macho_file)) { + if (!read_macho_file(TEST_FILE, &test_macho_file)) { LOG_ERROR("Failed to read macho_file"); } diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index f7362b6544..cb42862d73 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -22,6 +22,13 @@ class MachoTestFixture : public ::testing::Test { static uint32_t expected_symbol2_ind; static void SetUpTestSuite() { + bool fail = true; + section_info *expected_text_section = NULL; + section_info *expected_const_section = NULL; + section_info *expected_symbol_table = NULL; + section_info *expected_string_table = NULL; + section_info *expected_sections = NULL; + num_syms = 2; memcpy(expected_strtab, "__text\0__const\0symbol1\0symbol2\0", EXPECTED_STRTAB_SIZE); text_data[0] = 0xC3; @@ -30,7 +37,7 @@ class MachoTestFixture : public ::testing::Test { expected_symbol2_ind = 23; FILE *file = fopen(TEST_FILE, "wb"); - if (!file) { + if (file == NULL) { LOG_ERROR("Error with fopen() on %s file", TEST_FILE); } @@ -124,54 +131,85 @@ class MachoTestFixture : public ::testing::Test { // We use calloc for the below four calls to ensure that the untouched parts are zeroized, // as we will later memcmp the data to what we've read from the file. - section_info *expected_text_section = (section_info*) calloc(1, sizeof(section_info)); + expected_text_section = (section_info*) calloc(1, sizeof(section_info)); + if (expected_text_section == NULL) { + LOG_ERROR(" Error allocating memory for expected text section"); + goto end; + } strcpy(expected_text_section->name, "__text"); expected_text_section->size = text_section_size; expected_text_section->offset = text_section_offset; - section_info *expected_const_section = (section_info*) calloc(1, sizeof(section_info)); + expected_const_section = (section_info*) calloc(1, sizeof(section_info)); + if (expected_const_section == NULL) { + LOG_ERROR(" Error allocating memory for expected const section"); + goto end; + } strcpy(expected_const_section->name, "__const"); expected_const_section->size = const_section_size; expected_const_section->offset = const_section_offset; - section_info *expected_symbol_table = (section_info*) calloc(1, sizeof(section_info)); + expected_symbol_table = (section_info*) calloc(1, sizeof(section_info)); + if (expected_symbol_table == NULL) { + LOG_ERROR(" Error allocating memory for expected symbol table"); + goto end; + } strcpy(expected_symbol_table->name, "__symbol_table"); expected_symbol_table->size = num_syms * sizeof(symbol_info); expected_symbol_table->offset = symtab_command_symoff; - section_info *expected_string_table = (section_info*) calloc(1, sizeof(section_info)); + expected_string_table = (section_info*) calloc(1, sizeof(section_info)); + if (expected_string_table == NULL) { + LOG_ERROR(" Error allocating memory for expected string table"); + goto end; + } strcpy(expected_string_table->name, "__string_table"); expected_string_table->size = symtab_command_strsize; expected_string_table->offset = symtab_command_stroff; - section_info *expected_sections = (section_info*) malloc(sizeof(section_info) * 4); + expected_sections = (section_info*) malloc(sizeof(section_info) * 4); + if (expected_sections == NULL) { + LOG_ERROR("Error allocating memory for expected sections"); + goto end; + } memcpy(&expected_sections[0], expected_text_section, sizeof(section_info)); memcpy(&expected_sections[1], expected_const_section, sizeof(section_info)); memcpy(&expected_sections[2], expected_symbol_table, sizeof(section_info)); memcpy(&expected_sections[3], expected_string_table, sizeof(section_info)); - free(expected_text_section); - free(expected_const_section); - free(expected_symbol_table); - free(expected_string_table); - expected_macho = (machofile*) malloc(sizeof(machofile)); + if (expected_macho == NULL) { + LOG_ERROR("Error allocating memory for expected macho file struct"); + goto end; + } expected_macho->macho_header = test_header; expected_macho->num_sections = 4; expected_macho->sections = expected_sections; expected_symtab = (symbol_info*) malloc(num_syms * sizeof(symbol_info)); + if (expected_symtab == NULL) { + LOG_ERROR("Error allocating memory for expected symbol table struct"); + goto end; + } expected_symtab[0] = symbol1; expected_symtab[1] = symbol2; - } - static void TearDownTestSuite() { - if (expected_macho != NULL) { - free_macho_file(expected_macho); - } - if (expected_symtab != NULL) { + fail = false; +end: + if (fail) { + free(expected_sections); + free(expected_macho); free(expected_symtab); } + free(expected_text_section); + free(expected_const_section); + free(expected_symbol_table); + free(expected_string_table); + } + + static void TearDownTestSuite() { + free_macho_file(expected_macho); + free(expected_symtab); if (remove(TEST_FILE) != 0) { LOG_ERROR("Error deleting %s", TEST_FILE); } From 1d87e45266384ac558211b43439d848828db3e2f Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Tue, 27 Feb 2024 14:53:02 -0800 Subject: [PATCH 26/36] remove typedef aliases --- .../inject_hash/macho_parser/macho_parser.c | 18 ++++---- .../inject_hash/macho_parser/macho_parser.h | 9 +--- .../macho_parser/tests/macho_tests.cc | 4 +- .../macho_parser/tests/macho_tests.h | 44 +++++++++---------- 4 files changed, 34 insertions(+), 41 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index e9e927d0ee..72d44559c3 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -9,7 +9,7 @@ // Documentation for the Mach-O structs can be found in macho-o/loader.h and mach-o/nlist.h int read_macho_file(const char *filename, machofile *macho) { FILE *file = NULL; - load_cmd *load_commands = NULL; + struct load_command *load_commands = NULL; uint32_t bytes_read; int ret = 0; @@ -19,8 +19,8 @@ int read_macho_file(const char *filename, machofile *macho) { goto end; } - bytes_read = fread(&macho->macho_header, 1, sizeof(macho_header), file); - if (bytes_read != sizeof(macho_header)) { + bytes_read = fread(&macho->macho_header, 1, sizeof(struct mach_header_64), file); + if (bytes_read != sizeof(struct mach_header_64)) { LOG_ERROR("Error reading macho_header from file %s", filename); goto end; } @@ -50,9 +50,9 @@ int read_macho_file(const char *filename, machofile *macho) { uint32_t section_index = 0; for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { if (load_commands[i].cmd == LC_SEGMENT_64) { - segment_load_cmd *segment = (segment_load_cmd *)&load_commands[i]; + struct segment_command_64 *segment = (struct segment_command_64 *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { - section_data *sections = (section_data *)&segment[1]; + struct section_64 *sections = (struct section_64 *)&segment[1]; for (uint32_t j = 0; j < segment->nsects; j++) { if (strcmp(sections[j].sectname, "__text") == 0 || strcmp(sections[j].sectname, "__const") == 0) { macho->sections[section_index].offset = sections[j].offset; @@ -63,9 +63,9 @@ int read_macho_file(const char *filename, machofile *macho) { } } } else if (load_commands[i].cmd == LC_SYMTAB) { - symtab_load_cmd *symtab = (symtab_load_cmd *)&load_commands[i]; + struct symtab_command *symtab = (struct symtab_command *)&load_commands[i]; macho->sections[section_index].offset = symtab->symoff; - macho->sections[section_index].size = symtab->nsyms * sizeof(symbol_info); + macho->sections[section_index].size = symtab->nsyms * sizeof(struct nlist_64); strcpy(macho->sections[section_index].name, "__symbol_table"); section_index++; macho->sections[section_index].offset = symtab->stroff; @@ -157,8 +157,8 @@ uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table int found = 0; uint32_t index = 0; - for (uint32_t i = 0; i < symbol_table_size / sizeof(symbol_info); i++) { - symbol_info *symbol = (symbol_info *)(symbol_table_data + i * sizeof(symbol_info)); + for (uint32_t i = 0; i < symbol_table_size / sizeof(struct nlist_64); i++) { + struct nlist_64 *symbol = (struct nlist_64 *)(symbol_table_data + i * sizeof(struct nlist_64)); if (strcmp(symbol_name, &string_table[symbol->n_un.n_strx]) == 0) { if (found == 0) { index = symbol->n_value; diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index 8068cc4b4a..ce9fd28268 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -20,15 +20,8 @@ typedef struct { // Since we only support 64-bit architectures on Apple, we don't need to account for any of the 32-bit structures #define BIT_MODIFIER 8 -typedef struct mach_header_64 macho_header; -typedef struct load_command load_cmd; -typedef struct segment_command_64 segment_load_cmd; -typedef struct symtab_command symtab_load_cmd; -typedef struct section_64 section_data; -typedef struct nlist_64 symbol_info; - typedef struct { - macho_header macho_header; + struct mach_header_64 macho_header; section_info *sections; uint32_t num_sections; } machofile; diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc index 2017b997ad..2de3b73ee5 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc @@ -9,7 +9,7 @@ #define TEST_FILE "test_macho" machofile *MachoTestFixture::expected_macho; -symbol_info *MachoTestFixture::expected_symtab; +struct nlist_64 *MachoTestFixture::expected_symtab; uint32_t MachoTestFixture::num_syms; char MachoTestFixture::expected_strtab[EXPECTED_STRTAB_SIZE]; int MachoTestFixture::text_data[TEXT_DATA_SIZE]; @@ -23,7 +23,7 @@ TEST_F(MachoTestFixture, TestReadMachoFile) { LOG_ERROR("Failed to read macho_file"); } - EXPECT_TRUE(memcmp(&test_macho_file.macho_header, &expected_macho->macho_header, sizeof(macho_header)) == 0); + EXPECT_TRUE(memcmp(&test_macho_file.macho_header, &expected_macho->macho_header, sizeof(struct mach_header_64)) == 0); EXPECT_EQ(test_macho_file.num_sections, expected_macho->num_sections); EXPECT_TRUE(memcmp(test_macho_file.sections, expected_macho->sections, test_macho_file.num_sections * sizeof(section_info)) == 0); } diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index cb42862d73..d01865be08 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -13,7 +13,7 @@ class MachoTestFixture : public ::testing::Test { protected: static machofile *expected_macho; - static symbol_info *expected_symtab; + static struct nlist_64 *expected_symtab; static uint32_t num_syms; static char expected_strtab[EXPECTED_STRTAB_SIZE]; static int text_data[TEXT_DATA_SIZE]; @@ -41,26 +41,26 @@ class MachoTestFixture : public ::testing::Test { LOG_ERROR("Error with fopen() on %s file", TEST_FILE); } - uint32_t header_sizeofcmds = sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); + uint32_t header_sizeofcmds = sizeof(struct segment_command_64) + 2 * sizeof(struct section_64) + sizeof(struct symtab_command); uint32_t header_ncmds = 2; - macho_header test_header = { + struct mach_header_64 test_header = { .magic = MH_MAGIC_64, .ncmds = header_ncmds, .sizeofcmds = header_sizeofcmds, }; - uint32_t text_segment_cmdsize = sizeof(segment_load_cmd) + 2 * sizeof(section_data); + uint32_t text_segment_cmdsize = sizeof(struct segment_command_64) + 2 * sizeof(struct section_64); uint32_t text_segment_nsects = 2; - segment_load_cmd test_text_segment = { + struct segment_command_64 test_text_segment = { .cmd = LC_SEGMENT_64, .cmdsize = text_segment_cmdsize, .segname = "__TEXT", .nsects = text_segment_nsects, }; - uint32_t text_section_offset = sizeof(macho_header) + sizeof(segment_load_cmd) + 2 * sizeof(section_data) + sizeof(symtab_load_cmd); + uint32_t text_section_offset = sizeof(struct mach_header_64) + sizeof(struct segment_command_64) + 2 * sizeof(struct section_64) + sizeof(struct symtab_command); uint64_t text_section_size = TEXT_DATA_SIZE; // {0xC3} - section_data test_text_section = { + struct section_64 test_text_section = { .sectname = "__text", .size = text_section_size, .offset = text_section_offset, @@ -68,29 +68,29 @@ class MachoTestFixture : public ::testing::Test { uint32_t const_section_offset = text_section_offset + text_section_size; uint64_t const_section_size = CONST_DATA_SIZE; // "hi" - section_data test_const_section = { + struct section_64 test_const_section = { .sectname = "__const", .size = const_section_size, .offset = const_section_offset, }; uint32_t symtab_command_symoff = const_section_offset + const_section_size; - uint32_t symtab_command_stroff = symtab_command_symoff + num_syms * sizeof(symbol_info); + uint32_t symtab_command_stroff = symtab_command_symoff + num_syms * sizeof(struct nlist_64); uint32_t symtab_command_strsize = 32; - symtab_load_cmd test_symtab_command = { + struct symtab_command test_symtab_command = { .cmd = LC_SYMTAB, - .cmdsize = sizeof(symtab_load_cmd), + .cmdsize = sizeof(struct symtab_command), .symoff = symtab_command_symoff, .nsyms = num_syms, .stroff = symtab_command_stroff, .strsize = symtab_command_strsize, }; - fwrite(&test_header, sizeof(macho_header), 1, file); - fwrite(&test_text_segment, sizeof(segment_load_cmd), 1, file); - fwrite(&test_text_section, sizeof(section_data), 1, file); - fwrite(&test_const_section, sizeof(section_data), 1, file); - fwrite(&test_symtab_command, sizeof(symtab_load_cmd), 1, file); + fwrite(&test_header, sizeof(struct mach_header_64), 1, file); + fwrite(&test_text_segment, sizeof(struct segment_command_64), 1, file); + fwrite(&test_text_section, sizeof(struct section_64), 1, file); + fwrite(&test_const_section, sizeof(struct section_64), 1, file); + fwrite(&test_symtab_command, sizeof(struct symtab_command), 1, file); int *test_text_data = text_data; char *test_const_data = const_data; @@ -101,7 +101,7 @@ class MachoTestFixture : public ::testing::Test { fseek(file, test_const_section.offset, SEEK_SET); fwrite(test_const_data, sizeof(test_const_data), 1, file); - symbol_info symbol1 = { + struct nlist_64 symbol1 = { .n_un = {.n_strx = expected_symbol1_ind}, .n_type = 0, .n_sect = 1, @@ -109,7 +109,7 @@ class MachoTestFixture : public ::testing::Test { .n_value = expected_symbol1_ind, }; - symbol_info symbol2 = { + struct nlist_64 symbol2 = { .n_un = {.n_strx = expected_symbol2_ind}, .n_type = 0, .n_sect = 2, @@ -118,8 +118,8 @@ class MachoTestFixture : public ::testing::Test { }; fseek(file, symtab_command_symoff, SEEK_SET); - fwrite(&symbol1, sizeof(symbol_info), 1, file); - fwrite(&symbol2, sizeof(symbol_info), 1, file); + fwrite(&symbol1, sizeof(struct nlist_64), 1, file); + fwrite(&symbol2, sizeof(struct nlist_64), 1, file); // Write the string table fseek(file, symtab_command_stroff, SEEK_SET); @@ -155,7 +155,7 @@ class MachoTestFixture : public ::testing::Test { goto end; } strcpy(expected_symbol_table->name, "__symbol_table"); - expected_symbol_table->size = num_syms * sizeof(symbol_info); + expected_symbol_table->size = num_syms * sizeof(struct nlist_64); expected_symbol_table->offset = symtab_command_symoff; expected_string_table = (section_info*) calloc(1, sizeof(section_info)); @@ -186,7 +186,7 @@ class MachoTestFixture : public ::testing::Test { expected_macho->num_sections = 4; expected_macho->sections = expected_sections; - expected_symtab = (symbol_info*) malloc(num_syms * sizeof(symbol_info)); + expected_symtab = (struct nlist_64*) malloc(num_syms * sizeof(struct nlist_64)); if (expected_symtab == NULL) { LOG_ERROR("Error allocating memory for expected symbol table struct"); goto end; From 2d3360242d34c0719333475badb910ca74d43ff0 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Tue, 27 Feb 2024 15:25:16 -0800 Subject: [PATCH 27/36] add section_index counting --- .../fipstools/inject_hash/macho_parser/macho_parser.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 72d44559c3..252f538899 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -73,9 +73,18 @@ int read_macho_file(const char *filename, machofile *macho) { strcpy(macho->sections[section_index].name, "__string_table"); section_index++; } + + if (section_index > 4) { + LOG_ERROR("Duplicate sections found, %d", section_index); + goto end; + } + } + if (section_index != 4) { + LOG_ERROR("Not all required sections found"); + goto end; } - ret = 1; + ret = 1; end: free(load_commands); if (file != NULL) { From 1d1a567947fd75682f359a7c1bb76512b999168e Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Wed, 28 Feb 2024 10:24:58 -0800 Subject: [PATCH 28/36] programmatically find symbol indices in expected string table --- .../macho_parser/tests/macho_tests.h | 48 ++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index d01865be08..c607555bb6 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -21,6 +21,22 @@ class MachoTestFixture : public ::testing::Test { static uint32_t expected_symbol1_ind; static uint32_t expected_symbol2_ind; + static uint32_t FindSymbolIndex(const char *strtab, const char *symbol_name) { + const char *symbol = strtab; + uint32_t index = 0; + + while (*symbol != '\0') { + if (strcmp(symbol, symbol_name) == 0) { + return index; + } + + index += strlen(symbol) + 1; + symbol += strlen(symbol) + 1; + } + + return UINT32_MAX; + } + static void SetUpTestSuite() { bool fail = true; section_info *expected_text_section = NULL; @@ -29,12 +45,13 @@ class MachoTestFixture : public ::testing::Test { section_info *expected_string_table = NULL; section_info *expected_sections = NULL; + struct nlist_64 symbol1; + struct nlist_64 symbol2; + num_syms = 2; memcpy(expected_strtab, "__text\0__const\0symbol1\0symbol2\0", EXPECTED_STRTAB_SIZE); text_data[0] = 0xC3; memcpy(const_data, "hi", CONST_DATA_SIZE); - expected_symbol1_ind = 15; - expected_symbol2_ind = 23; FILE *file = fopen(TEST_FILE, "wb"); if (file == NULL) { @@ -100,16 +117,26 @@ class MachoTestFixture : public ::testing::Test { fseek(file, test_const_section.offset, SEEK_SET); fwrite(test_const_data, sizeof(test_const_data), 1, file); - - struct nlist_64 symbol1 = { - .n_un = {.n_strx = expected_symbol1_ind}, - .n_type = 0, - .n_sect = 1, - .n_desc = 0, - .n_value = expected_symbol1_ind, + + expected_symbol1_ind = FindSymbolIndex(expected_strtab, "symbol1"); + if (expected_symbol1_ind == UINT32_MAX) { + LOG_ERROR("symbol1 not found in expected string table"); + goto end; + } + symbol1 = { + .n_un = {.n_strx = expected_symbol1_ind}, + .n_type = 0, + .n_sect = 1, + .n_desc = 0, + .n_value = expected_symbol1_ind, }; - struct nlist_64 symbol2 = { + expected_symbol2_ind = FindSymbolIndex(expected_strtab, "symbol2"); + if (expected_symbol2_ind == UINT32_MAX) { + LOG_ERROR("symbol2 not found in expected string table"); + goto end; + } + symbol2 = { .n_un = {.n_strx = expected_symbol2_ind}, .n_type = 0, .n_sect = 2, @@ -121,7 +148,6 @@ class MachoTestFixture : public ::testing::Test { fwrite(&symbol1, sizeof(struct nlist_64), 1, file); fwrite(&symbol2, sizeof(struct nlist_64), 1, file); - // Write the string table fseek(file, symtab_command_stroff, SEEK_SET); fwrite(expected_strtab, symtab_command_strsize, 1, file); From f507f5132c1c5ee5941db9d660f288346736d1cd Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Wed, 28 Feb 2024 11:09:02 -0800 Subject: [PATCH 29/36] avoid using memcpy to assign string to arrays --- .../macho_parser/tests/macho_tests.cc | 8 ++--- .../macho_parser/tests/macho_tests.h | 34 +++++++------------ 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc index 2de3b73ee5..4a34985075 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc @@ -10,13 +10,13 @@ machofile *MachoTestFixture::expected_macho; struct nlist_64 *MachoTestFixture::expected_symtab; -uint32_t MachoTestFixture::num_syms; -char MachoTestFixture::expected_strtab[EXPECTED_STRTAB_SIZE]; -int MachoTestFixture::text_data[TEXT_DATA_SIZE]; -char MachoTestFixture::const_data[CONST_DATA_SIZE]; uint32_t MachoTestFixture::expected_symbol1_ind; uint32_t MachoTestFixture::expected_symbol2_ind; +constexpr char MachoTestFixture::expected_strtab[EXPECTED_STRTAB_SIZE]; +constexpr int MachoTestFixture::text_data[TEXT_DATA_SIZE]; +constexpr char MachoTestFixture::const_data[CONST_DATA_SIZE]; + TEST_F(MachoTestFixture, TestReadMachoFile) { machofile test_macho_file; if (!read_macho_file(TEST_FILE, &test_macho_file)) { diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index c607555bb6..3aaaa91d0c 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -6,18 +6,18 @@ #include "../macho_parser.h" #define TEST_FILE "test_macho" -#define EXPECTED_STRTAB_SIZE 31 +#define EXPECTED_STRTAB_SIZE 32 #define TEXT_DATA_SIZE 1 -#define CONST_DATA_SIZE 2 +#define CONST_DATA_SIZE 3 +#define NUM_SYMS 2 class MachoTestFixture : public ::testing::Test { protected: static machofile *expected_macho; static struct nlist_64 *expected_symtab; - static uint32_t num_syms; - static char expected_strtab[EXPECTED_STRTAB_SIZE]; - static int text_data[TEXT_DATA_SIZE]; - static char const_data[CONST_DATA_SIZE]; + static constexpr char expected_strtab[] = "__text\0__const\0symbol1\0symbol2\0"; + static constexpr int text_data[] = { 0xC3 }; + static constexpr char const_data[] = "hi"; static uint32_t expected_symbol1_ind; static uint32_t expected_symbol2_ind; @@ -48,12 +48,7 @@ class MachoTestFixture : public ::testing::Test { struct nlist_64 symbol1; struct nlist_64 symbol2; - num_syms = 2; - memcpy(expected_strtab, "__text\0__const\0symbol1\0symbol2\0", EXPECTED_STRTAB_SIZE); - text_data[0] = 0xC3; - memcpy(const_data, "hi", CONST_DATA_SIZE); - - FILE *file = fopen(TEST_FILE, "wb"); + static FILE *file = fopen(TEST_FILE, "wb"); if (file == NULL) { LOG_ERROR("Error with fopen() on %s file", TEST_FILE); } @@ -92,13 +87,13 @@ class MachoTestFixture : public ::testing::Test { }; uint32_t symtab_command_symoff = const_section_offset + const_section_size; - uint32_t symtab_command_stroff = symtab_command_symoff + num_syms * sizeof(struct nlist_64); + uint32_t symtab_command_stroff = symtab_command_symoff + NUM_SYMS * sizeof(struct nlist_64); uint32_t symtab_command_strsize = 32; struct symtab_command test_symtab_command = { .cmd = LC_SYMTAB, .cmdsize = sizeof(struct symtab_command), .symoff = symtab_command_symoff, - .nsyms = num_syms, + .nsyms = NUM_SYMS, .stroff = symtab_command_stroff, .strsize = symtab_command_strsize, }; @@ -109,14 +104,11 @@ class MachoTestFixture : public ::testing::Test { fwrite(&test_const_section, sizeof(struct section_64), 1, file); fwrite(&test_symtab_command, sizeof(struct symtab_command), 1, file); - int *test_text_data = text_data; - char *test_const_data = const_data; - fseek(file, test_text_section.offset, SEEK_SET); - fwrite(test_text_data, sizeof(test_text_data), 1, file); + fwrite(text_data, sizeof(text_data), 1, file); fseek(file, test_const_section.offset, SEEK_SET); - fwrite(test_const_data, sizeof(test_const_data), 1, file); + fwrite(const_data, sizeof(const_data), 1, file); expected_symbol1_ind = FindSymbolIndex(expected_strtab, "symbol1"); if (expected_symbol1_ind == UINT32_MAX) { @@ -181,7 +173,7 @@ class MachoTestFixture : public ::testing::Test { goto end; } strcpy(expected_symbol_table->name, "__symbol_table"); - expected_symbol_table->size = num_syms * sizeof(struct nlist_64); + expected_symbol_table->size = NUM_SYMS * sizeof(struct nlist_64); expected_symbol_table->offset = symtab_command_symoff; expected_string_table = (section_info*) calloc(1, sizeof(section_info)); @@ -212,7 +204,7 @@ class MachoTestFixture : public ::testing::Test { expected_macho->num_sections = 4; expected_macho->sections = expected_sections; - expected_symtab = (struct nlist_64*) malloc(num_syms * sizeof(struct nlist_64)); + expected_symtab = (struct nlist_64*) malloc(NUM_SYMS * sizeof(struct nlist_64)); if (expected_symtab == NULL) { LOG_ERROR("Error allocating memory for expected symbol table struct"); goto end; From 15f8083258dc290470ce300ba41016a4e25b40ab Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Wed, 28 Feb 2024 11:21:21 -0800 Subject: [PATCH 30/36] avoid memory leakage in tests --- .../macho_parser/tests/macho_tests.cc | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc index 4a34985075..38c921ba12 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.cc @@ -29,40 +29,38 @@ TEST_F(MachoTestFixture, TestReadMachoFile) { } TEST_F(MachoTestFixture, TestGetMachoSectionData) { - uint8_t *text_section = NULL; - size_t text_section_size; + std::unique_ptr text_section(nullptr); + std::unique_ptr const_section(nullptr); + std::unique_ptr symbol_table(nullptr); + std::unique_ptr string_table(nullptr); - uint8_t *const_section = NULL; + size_t text_section_size; size_t const_section_size; - - uint8_t *symbol_table = NULL; size_t symbol_table_size; - - uint8_t *string_table = NULL; size_t string_table_size; - text_section = get_macho_section_data(TEST_FILE, expected_macho, "__text", &text_section_size, NULL); - const_section = get_macho_section_data(TEST_FILE, expected_macho, "__const", &const_section_size, NULL); - symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); - string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); + text_section.reset(get_macho_section_data(TEST_FILE, expected_macho, "__text", &text_section_size, NULL)); + const_section.reset(get_macho_section_data(TEST_FILE, expected_macho, "__const", &const_section_size, NULL)); + symbol_table.reset(get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL)); + string_table.reset(get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL)); - ASSERT_TRUE(memcmp(text_section, text_data, text_section_size) == 0); - ASSERT_TRUE(memcmp(const_section, const_data, const_section_size) == 0); - ASSERT_TRUE(memcmp(symbol_table, expected_symtab, symbol_table_size) == 0); - ASSERT_TRUE(memcmp(string_table, expected_strtab, string_table_size) == 0); + ASSERT_TRUE(memcmp(text_section.get(), text_data, text_section_size) == 0); + ASSERT_TRUE(memcmp(const_section.get(), const_data, const_section_size) == 0); + ASSERT_TRUE(memcmp(symbol_table.get(), expected_symtab, symbol_table_size) == 0); + ASSERT_TRUE(memcmp(string_table.get(), expected_strtab, string_table_size) == 0); } TEST_F(MachoTestFixture, TestFindMachoSymbolIndex) { - uint8_t *symbol_table = NULL; - size_t symbol_table_size; + std::unique_ptr symbol_table(nullptr); + std::unique_ptr string_table(nullptr); - uint8_t *string_table = NULL; + size_t symbol_table_size; size_t string_table_size; - symbol_table = get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL); - string_table = get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL); + symbol_table.reset(get_macho_section_data(TEST_FILE, expected_macho, "__symbol_table", &symbol_table_size, NULL)); + string_table.reset(get_macho_section_data(TEST_FILE, expected_macho, "__string_table", &string_table_size, NULL)); - uint32_t symbol1_index = find_macho_symbol_index(symbol_table, symbol_table_size, string_table, string_table_size, "symbol1", NULL); + uint32_t symbol1_index = find_macho_symbol_index(symbol_table.get(), symbol_table_size, string_table.get(), string_table_size, "symbol1", NULL); ASSERT_EQ(symbol1_index, expected_symbol1_ind); } From 01cb2260a9489dd79bfec170e226716e2c3d91eb Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Wed, 28 Feb 2024 14:56:04 -0800 Subject: [PATCH 31/36] correct return value --- util/fipstools/inject_hash/macho_parser/macho_parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 252f538899..c6e0d95cc5 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -150,7 +150,7 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table_size, uint8_t *string_table_data, size_t string_table_size, const char *symbol_name, uint32_t *base) { char* string_table = NULL; - int ret = 0; + uint32_t ret = 0; if (symbol_table_data == NULL || string_table_data == NULL) { LOG_ERROR("Symbol and string table pointers cannot be null to find the symbol index"); From a5642216d281eeb728a02193c021d37256894450 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 14 Mar 2024 12:46:31 -0700 Subject: [PATCH 32/36] add missing error handling --- .../inject_hash/macho_parser/macho_parser.c | 13 ++-- .../macho_parser/tests/macho_tests.h | 71 +++++++++++++++---- 2 files changed, 64 insertions(+), 20 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index c6e0d95cc5..6204b8970f 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -101,7 +101,6 @@ void free_macho_file(machofile *macho) { uint8_t* get_macho_section_data(const char *filename, machofile *macho, const char *section_name, size_t *size, uint32_t *offset) { FILE *file = NULL; - uint8_t *section_data = NULL; uint8_t *ret = NULL; uint32_t bytes_read; @@ -112,15 +111,20 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch } for (uint32_t i = 0; i < macho->num_sections; i++) { if (strcmp(macho->sections[i].name, section_name) == 0) { - section_data = malloc(macho->sections[i].size); + uint8_t *section_data = malloc(macho->sections[i].size); if (section_data == NULL) { LOG_ERROR("Error allocating memory for section data"); goto end; } - fseek(file, macho->sections[i].offset, SEEK_SET); + if (fseek(file, macho->sections[i].offset, SEEK_SET) != 0) { + free(section_data); + LOG_ERROR("Failed to seek in file %s", filename); + goto end; + } bytes_read = fread(section_data, 1, macho->sections[i].size, file); if (bytes_read != macho->sections[i].size) { + free(section_data); LOG_ERROR("Error reading section data from file %s", filename); goto end; } @@ -142,9 +146,6 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch if (file != NULL) { fclose(file); } - if (ret == NULL) { - free(section_data); - } return ret; } diff --git a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h index 3aaaa91d0c..33783f60a5 100644 --- a/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h +++ b/util/fipstools/inject_hash/macho_parser/tests/macho_tests.h @@ -98,17 +98,44 @@ class MachoTestFixture : public ::testing::Test { .strsize = symtab_command_strsize, }; - fwrite(&test_header, sizeof(struct mach_header_64), 1, file); - fwrite(&test_text_segment, sizeof(struct segment_command_64), 1, file); - fwrite(&test_text_section, sizeof(struct section_64), 1, file); - fwrite(&test_const_section, sizeof(struct section_64), 1, file); - fwrite(&test_symtab_command, sizeof(struct symtab_command), 1, file); + if (fwrite(&test_header, sizeof(struct mach_header_64), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } + if (fwrite(&test_text_segment, sizeof(struct segment_command_64), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } + if (fwrite(&test_text_section, sizeof(struct section_64), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } + if (fwrite(&test_const_section, sizeof(struct section_64), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } + if (fwrite(&test_symtab_command, sizeof(struct symtab_command), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } - fseek(file, test_text_section.offset, SEEK_SET); - fwrite(text_data, sizeof(text_data), 1, file); + if (fseek(file, test_text_section.offset, SEEK_SET) != 0) { + LOG_ERROR("Failed to seek in file %s", TEST_FILE); + goto end; + } + if (fwrite(text_data, sizeof(text_data), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } - fseek(file, test_const_section.offset, SEEK_SET); - fwrite(const_data, sizeof(const_data), 1, file); + if (fseek(file, test_const_section.offset, SEEK_SET) != 0) { + LOG_ERROR("Failed to seek in file %s", TEST_FILE); + goto end; + } + if (fwrite(const_data, sizeof(const_data), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } expected_symbol1_ind = FindSymbolIndex(expected_strtab, "symbol1"); if (expected_symbol1_ind == UINT32_MAX) { @@ -136,15 +163,31 @@ class MachoTestFixture : public ::testing::Test { .n_value = expected_symbol2_ind, }; - fseek(file, symtab_command_symoff, SEEK_SET); - fwrite(&symbol1, sizeof(struct nlist_64), 1, file); - fwrite(&symbol2, sizeof(struct nlist_64), 1, file); + if (fseek(file, symtab_command_symoff, SEEK_SET) != 0) { + LOG_ERROR("Failed to seek in file %s", TEST_FILE); + goto end; + } + if (fwrite(&symbol1, sizeof(struct nlist_64), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } + if (fwrite(&symbol2, sizeof(struct nlist_64), 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } - fseek(file, symtab_command_stroff, SEEK_SET); - fwrite(expected_strtab, symtab_command_strsize, 1, file); + if (fseek(file, symtab_command_stroff, SEEK_SET) != 0) { + LOG_ERROR("Failed to seek in file %s", TEST_FILE); + goto end; + } + if (fwrite(expected_strtab, symtab_command_strsize, 1, file) != 1) { + LOG_ERROR("Error occurred while writing to file %s", TEST_FILE); + goto end; + } if (fclose(file) != 0) { LOG_ERROR("Error closing file\n"); + goto end; } // We use calloc for the below four calls to ensure that the untouched parts are zeroized, From be280e9bfc1f0792fcb4ea03d4a04b2ef5de8e3e Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 14 Mar 2024 13:34:24 -0700 Subject: [PATCH 33/36] use set indices for sections we're looking for --- .../inject_hash/macho_parser/macho_parser.c | 126 +++++++++++------- 1 file changed, 75 insertions(+), 51 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 6204b8970f..1d14144e2a 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -6,6 +6,11 @@ #include "common.h" #include "macho_parser.h" +#define TEXT_INDEX 0 +#define CONST_INDEX 1 +#define SYMTABLE_INDEX 2 +#define STRTABLE_INDEX 3 + // Documentation for the Mach-O structs can be found in macho-o/loader.h and mach-o/nlist.h int read_macho_file(const char *filename, machofile *macho) { FILE *file = NULL; @@ -47,41 +52,51 @@ int read_macho_file(const char *filename, machofile *macho) { LOG_ERROR("Error allocating memory for macho sections"); } - uint32_t section_index = 0; + int text_found = 0; + int const_found = 0; + int symtab_found = 0; + for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { if (load_commands[i].cmd == LC_SEGMENT_64) { struct segment_command_64 *segment = (struct segment_command_64 *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { struct section_64 *sections = (struct section_64 *)&segment[1]; for (uint32_t j = 0; j < segment->nsects; j++) { - if (strcmp(sections[j].sectname, "__text") == 0 || strcmp(sections[j].sectname, "__const") == 0) { - macho->sections[section_index].offset = sections[j].offset; - macho->sections[section_index].size = sections[j].size; - strcpy(macho->sections[section_index].name, sections[j].sectname); - section_index++; + if (strcmp(sections[j].sectname, "__text") == 0) { + if (text_found == 1) { + LOG_ERROR("Duplicate __text section found"); + goto end; + } + macho->sections[TEXT_INDEX].offset = sections[j].offset; + macho->sections[TEXT_INDEX].size = sections[j].size; + strcpy(macho->sections[TEXT_INDEX].name, sections[j].sectname); + text_found = 1; + } else if (strcmp(sections[j].sectname, "__const") == 0) { + if (const_found == 1) { + LOG_ERROR("Duplicate __const section found"); + goto end; + } + macho->sections[CONST_INDEX].offset = sections[j].offset; + macho->sections[CONST_INDEX].size = sections[j].size; + strcpy(macho->sections[CONST_INDEX].name, sections[j].sectname); + const_found = 1; } } } } else if (load_commands[i].cmd == LC_SYMTAB) { + if (symtab_found == 1) { + LOG_ERROR("Duplicate symbol and string tables found"); + goto end; + } struct symtab_command *symtab = (struct symtab_command *)&load_commands[i]; - macho->sections[section_index].offset = symtab->symoff; - macho->sections[section_index].size = symtab->nsyms * sizeof(struct nlist_64); - strcpy(macho->sections[section_index].name, "__symbol_table"); - section_index++; - macho->sections[section_index].offset = symtab->stroff; - macho->sections[section_index].size = symtab->strsize; - strcpy(macho->sections[section_index].name, "__string_table"); - section_index++; + macho->sections[SYMTABLE_INDEX].offset = symtab->symoff; + macho->sections[SYMTABLE_INDEX].size = symtab->nsyms * sizeof(struct nlist_64); + strcpy(macho->sections[SYMTABLE_INDEX].name, "__symbol_table"); + macho->sections[STRTABLE_INDEX].offset = symtab->stroff; + macho->sections[STRTABLE_INDEX].size = symtab->strsize; + strcpy(macho->sections[STRTABLE_INDEX].name, "__string_table"); + symtab_found = 1; } - - if (section_index > 4) { - LOG_ERROR("Duplicate sections found, %d", section_index); - goto end; - } - } - if (section_index != 4) { - LOG_ERROR("Not all required sections found"); - goto end; } ret = 1; @@ -109,39 +124,48 @@ uint8_t* get_macho_section_data(const char *filename, machofile *macho, const ch LOG_ERROR("Error opening file %s", filename); goto end; } - for (uint32_t i = 0; i < macho->num_sections; i++) { - if (strcmp(macho->sections[i].name, section_name) == 0) { - uint8_t *section_data = malloc(macho->sections[i].size); - if (section_data == NULL) { - LOG_ERROR("Error allocating memory for section data"); - goto end; - } - if (fseek(file, macho->sections[i].offset, SEEK_SET) != 0) { - free(section_data); - LOG_ERROR("Failed to seek in file %s", filename); - goto end; - } - bytes_read = fread(section_data, 1, macho->sections[i].size, file); - if (bytes_read != macho->sections[i].size) { - free(section_data); - LOG_ERROR("Error reading section data from file %s", filename); - goto end; - } + int section_index; + if (strcmp(section_name, "__text") == 0) { + section_index = TEXT_INDEX; + } else if (strcmp(section_name, "__const") == 0) { + section_index = CONST_INDEX; + } else if (strcmp(section_name, "__symbol_table") == 0) { + section_index = SYMTABLE_INDEX; + } else if (strcmp(section_name, "__string_table") == 0) { + section_index = STRTABLE_INDEX; + } else { + LOG_ERROR("Getting invalid macho section data %s", section_name); + goto end; + } - if (size != NULL) { - *size = macho->sections[i].size; - } - if (offset != NULL) { - *offset = macho->sections[i].offset; - } + uint8_t *section_data = malloc(macho->sections[section_index].size); + if (section_data == NULL) { + LOG_ERROR("Error allocating memory for section data"); + goto end; + } - ret = section_data; - goto end; - } + if (fseek(file, macho->sections[section_index].offset, SEEK_SET) != 0) { + free(section_data); + LOG_ERROR("Failed to seek in file %s", filename); + goto end; + } + bytes_read = fread(section_data, 1, macho->sections[section_index].size, file); + if (bytes_read != macho->sections[section_index].size) { + free(section_data); + LOG_ERROR("Error reading section data from file %s", filename); + goto end; } - LOG_ERROR("Section %s not found in macho file %s", section_name, filename); + if (size != NULL) { + *size = macho->sections[section_index].size; + } + if (offset != NULL) { + *offset = macho->sections[section_index].offset; + } + + ret = section_data; + end: if (file != NULL) { fclose(file); From 638ff8bf1ebb392b9097adaaececb278f656d807 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 14 Mar 2024 15:18:25 -0700 Subject: [PATCH 34/36] remove unnecessary macro --- util/fipstools/inject_hash/macho_parser/macho_parser.c | 2 +- util/fipstools/inject_hash/macho_parser/macho_parser.h | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 1d14144e2a..5a0958bcf6 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -56,7 +56,7 @@ int read_macho_file(const char *filename, machofile *macho) { int const_found = 0; int symtab_found = 0; - for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / BIT_MODIFIER; i += load_commands[i].cmdsize / BIT_MODIFIER) { + for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / sizeof(struct load_command); i += load_commands[i].cmdsize / sizeof(struct load_command)) { if (load_commands[i].cmd == LC_SEGMENT_64) { struct segment_command_64 *segment = (struct segment_command_64 *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.h b/util/fipstools/inject_hash/macho_parser/macho_parser.h index ce9fd28268..a11463fc87 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.h +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.h @@ -17,9 +17,6 @@ typedef struct { uint32_t offset; } section_info; -// Since we only support 64-bit architectures on Apple, we don't need to account for any of the 32-bit structures -#define BIT_MODIFIER 8 - typedef struct { struct mach_header_64 macho_header; section_info *sections; From d9e32340f21a8bb313c2dcd71d52bd7dfc1fe50a Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Thu, 21 Mar 2024 10:40:46 -0700 Subject: [PATCH 35/36] add comment explaining why load_command search works --- util/fipstools/inject_hash/macho_parser/macho_parser.c | 1 + 1 file changed, 1 insertion(+) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 5a0958bcf6..04cd5b320a 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -56,6 +56,7 @@ int read_macho_file(const char *filename, machofile *macho) { int const_found = 0; int symtab_found = 0; + // mach-o/loader.h explains that cmdsize (and by extension sizeofcmds) must be a multiple of 8 on 64-bit systems. struct load_command will always be 8 bytes. for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / sizeof(struct load_command); i += load_commands[i].cmdsize / sizeof(struct load_command)) { if (load_commands[i].cmd == LC_SEGMENT_64) { struct segment_command_64 *segment = (struct segment_command_64 *)&load_commands[i]; From a9a1d4cd99a25070307fd894c76513242f540408 Mon Sep 17 00:00:00 2001 From: Bill Yang Date: Wed, 10 Apr 2024 10:36:27 -0700 Subject: [PATCH 36/36] use size_t where appropriate --- util/fipstools/inject_hash/macho_parser/macho_parser.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/util/fipstools/inject_hash/macho_parser/macho_parser.c b/util/fipstools/inject_hash/macho_parser/macho_parser.c index 04cd5b320a..b24c343403 100644 --- a/util/fipstools/inject_hash/macho_parser/macho_parser.c +++ b/util/fipstools/inject_hash/macho_parser/macho_parser.c @@ -57,12 +57,12 @@ int read_macho_file(const char *filename, machofile *macho) { int symtab_found = 0; // mach-o/loader.h explains that cmdsize (and by extension sizeofcmds) must be a multiple of 8 on 64-bit systems. struct load_command will always be 8 bytes. - for (uint32_t i = 0; i < macho->macho_header.sizeofcmds / sizeof(struct load_command); i += load_commands[i].cmdsize / sizeof(struct load_command)) { + for (size_t i = 0; i < macho->macho_header.sizeofcmds / sizeof(struct load_command); i += load_commands[i].cmdsize / sizeof(struct load_command)) { if (load_commands[i].cmd == LC_SEGMENT_64) { struct segment_command_64 *segment = (struct segment_command_64 *)&load_commands[i]; if (strcmp(segment->segname, "__TEXT") == 0) { struct section_64 *sections = (struct section_64 *)&segment[1]; - for (uint32_t j = 0; j < segment->nsects; j++) { + for (size_t j = 0; j < segment->nsects; j++) { if (strcmp(sections[j].sectname, "__text") == 0) { if (text_found == 1) { LOG_ERROR("Duplicate __text section found"); @@ -191,8 +191,8 @@ uint32_t find_macho_symbol_index(uint8_t *symbol_table_data, size_t symbol_table memcpy(string_table, string_table_data, string_table_size); int found = 0; - uint32_t index = 0; - for (uint32_t i = 0; i < symbol_table_size / sizeof(struct nlist_64); i++) { + size_t index = 0; + for (size_t i = 0; i < symbol_table_size / sizeof(struct nlist_64); i++) { struct nlist_64 *symbol = (struct nlist_64 *)(symbol_table_data + i * sizeof(struct nlist_64)); if (strcmp(symbol_name, &string_table[symbol->n_un.n_strx]) == 0) { if (found == 0) {