From 570286220e28e606e199b37a06cd199cadb592ba Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Tue, 3 Feb 2015 20:42:36 +0000 Subject: [PATCH] Fix memory access violations triggered by running readelf on fuzzed binaries. PR binutils/17531 * dwarf.c (process_debug_info): Add range check. (display_debug_pubnames_worker): Likewise. (display_gdb_index): Fix range check. (process_cu_tu_index): Add range check. * readelf.c (get_data): Change parameter types from size_t to bfd_size_type. Add checks for loss of accuracy when casting from bfd_size_type to size_t. (get_dynamic_data): Likewise. (process_section_groups): Limit number of error messages. --- binutils/ChangeLog | 13 +++++++ binutils/dwarf.c | 30 ++++++++++++--- binutils/readelf.c | 107 ++++++++++++++++++++++++++++++++++++++--------------- 3 files changed, 114 insertions(+), 36 deletions(-) diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 4a6de8d..4e5f4f2 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,5 +1,18 @@ 2015-02-03 Nick Clifton + PR binutils/17531 + * dwarf.c (process_debug_info): Add range check. + (display_debug_pubnames_worker): Likewise. + (display_gdb_index): Fix range check. + (process_cu_tu_index): Add range check. + * readelf.c (get_data): Change parameter types from size_t to + bfd_size_type. Add checks for loss of accuracy when casting from + bfd_size_type to size_t. + (get_dynamic_data): Likewise. + (process_section_groups): Limit number of error messages. + +2015-02-03 Nick Clifton + PR binutils/17512 * objdump.c (display_any_bfd): Fail if archives nest too deeply. diff --git a/binutils/dwarf.c b/binutils/dwarf.c index d82c89c..bee8b64 100644 --- a/binutils/dwarf.c +++ b/binutils/dwarf.c @@ -2444,11 +2444,19 @@ process_debug_info (struct dwarf_section *section, " extends beyond end of section (length = %s)\n"), dwarf_vmatoa ("x", cu_offset), dwarf_vmatoa ("x", compunit.cu_length)); + num_units = unit; break; } tags = hdrptr; start += compunit.cu_length + initial_length_size; + if (start > end) + { + warn (_("Debug info is corrupt. CU at %s extends beyond end of section"), + dwarf_vmatoa ("x", cu_offset)); + start = end; + } + if (compunit.cu_version != 2 && compunit.cu_version != 3 && compunit.cu_version != 4) @@ -3720,7 +3728,9 @@ display_debug_pubnames_worker (struct dwarf_section *section, SAFE_BYTE_GET_AND_INC (names.pn_size, data, offset_size, end); /* PR 17531: file: 7615b6b2. */ - if ((dwarf_signed_vma) names.pn_length < 0) + if ((dwarf_signed_vma) names.pn_length < 0 + /* PR 17531: file: a5dbeaa7. */ + || start + names.pn_length + initial_length_size < start) { warn (_("Negative length for public name: 0x%lx\n"), (long) names.pn_length); start = end; @@ -6691,19 +6701,19 @@ display_gdb_index (struct dwarf_section *section, constant_pool + name_offset); if (constant_pool + cu_vector_offset < constant_pool - || constant_pool + cu_vector_offset >= section->start + section->size) + || constant_pool + cu_vector_offset >= section->start + section->size - 3) { printf (_("\n"), cu_vector_offset); warn (_("Corrupt CU vector offset of 0x%x found for symbol table slot %d\n"), cu_vector_offset, i); continue; } - else - num_cus = byte_get_little_endian (constant_pool + cu_vector_offset, 4); + + num_cus = byte_get_little_endian (constant_pool + cu_vector_offset, 4); if (num_cus * 4 < num_cus - || constant_pool + cu_vector_offset + 4 + num_cus * 4 >= - section->start + section->size) + || constant_pool + cu_vector_offset + 4 + num_cus * 4 + >= section->start + section->size) { printf ("\n", num_cus); warn (_("Invalid number of CUs (0x%x) for symbol table slot %d\n"), @@ -6864,6 +6874,14 @@ process_cu_tu_index (struct dwarf_section *section, int do_display) pindex = phash + nslots * 8; ppool = pindex + nslots * 4; + /* PR 17531: file: 45d69832. */ + if (pindex < phash || ppool < phdr) + { + warn (_("Section %s is too small for %d slots\n"), + section->name, nslots); + return 0; + } + if (do_display) { printf (_("Contents of the %s section:\n\n"), section->name); diff --git a/binutils/readelf.c b/binutils/readelf.c index e08d74e..a0d6f32 100644 --- a/binutils/readelf.c +++ b/binutils/readelf.c @@ -324,23 +324,45 @@ static const char *get_symbol_version_string context. */ static void * -get_data (void * var, FILE * file, unsigned long offset, size_t size, size_t nmemb, - const char * reason) +get_data (void * var, FILE * file, unsigned long offset, bfd_size_type size, + bfd_size_type nmemb, const char * reason) { void * mvar; - size_t amt = size * nmemb; + bfd_size_type amt = size * nmemb; if (size == 0 || nmemb == 0) return NULL; + /* If the size_t type is smaller than the bfd_size_type, eg because + you are building a 32-bit tool on a 64-bit host, then make sure + that when the sizes are cast to (size_t) no information is lost. */ + if (sizeof (size_t) < sizeof (bfd_size_type) + && ( (bfd_size_type) ((size_t) size) != size + || (bfd_size_type) ((size_t) nmemb) != nmemb)) + { + if (reason) + error (_("Size truncation prevents reading 0x%llx elements of size 0x%llx for %s\n"), + (unsigned long long) nmemb, (unsigned long long) size, reason); + return NULL; + } + + /* Check for size overflow. */ + if (amt < nmemb) + { + if (reason) + error (_("Size overflow prevents reading 0x%llx elements of size 0x%llx for %s\n"), + (unsigned long long) nmemb, (unsigned long long) size, reason); + return NULL; + } + /* Be kind to memory chekers (eg valgrind, address sanitizer) by not attempting to allocate memory when the read is bound to fail. */ if (amt > current_file_size || offset + archive_file_offset + amt > current_file_size) { if (reason) - error (_("Reading 0x%lx bytes extends past end of file for %s\n"), - (unsigned long) amt, reason); + error (_("Reading 0x%llx bytes extends past end of file for %s\n"), + (unsigned long long) amt, reason); return NULL; } @@ -356,26 +378,26 @@ get_data (void * var, FILE * file, unsigned long offset, size_t size, size_t nme if (mvar == NULL) { /* Check for overflow. */ - if (nmemb < (~(size_t) 0 - 1) / size) + if (nmemb < (~(bfd_size_type) 0 - 1) / size) /* + 1 so that we can '\0' terminate invalid string table sections. */ - mvar = malloc (size * nmemb + 1); + mvar = malloc ((size_t) amt + 1); if (mvar == NULL) { if (reason) - error (_("Out of memory allocating 0x%lx bytes for %s\n"), - (unsigned long)(size * nmemb), reason); + error (_("Out of memory allocating 0x%llx bytes for %s\n"), + (unsigned long long) amt, reason); return NULL; } ((char *) mvar)[amt] = '\0'; } - if (fread (mvar, size, nmemb, file) != nmemb) + if (fread (mvar, (size_t) size, (size_t) nmemb, file) != nmemb) { if (reason) - error (_("Unable to read in 0x%lx bytes of %s\n"), - (unsigned long) amt, reason); + error (_("Unable to read in 0x%llx bytes of %s\n"), + (unsigned long long) amt, reason); if (mvar != var) free (mvar); return NULL; @@ -5994,8 +6016,15 @@ process_section_groups (FILE * file) if (entry >= elf_header.e_shnum) { - error (_("section [%5u] in group section [%5u] > maximum section [%5u]\n"), - entry, i, elf_header.e_shnum - 1); + static unsigned num_group_errors = 0; + + if (num_group_errors ++ < 10) + { + error (_("section [%5u] in group section [%5u] > maximum section [%5u]\n"), + entry, i, elf_header.e_shnum - 1); + if (num_group_errors == 10) + warn (_("Futher error messages about overlarge group section indicies suppressed\n")); + } continue; } @@ -6003,9 +6032,16 @@ process_section_groups (FILE * file) { if (entry) { - error (_("section [%5u] in group section [%5u] already in group section [%5u]\n"), - entry, i, - section_headers_groups [entry]->group_index); + static unsigned num_errs = 0; + + if (num_errs ++ < 10) + { + error (_("section [%5u] in group section [%5u] already in group section [%5u]\n"), + entry, i, + section_headers_groups [entry]->group_index); + if (num_errs == 10) + warn (_("Further error messages about already contained group sections suppressed\n")); + } continue; } else @@ -10051,41 +10087,52 @@ get_symbol_index_type (unsigned int type) } static bfd_vma * -get_dynamic_data (FILE * file, size_t number, unsigned int ent_size) +get_dynamic_data (FILE * file, bfd_size_type number, unsigned int ent_size) { unsigned char * e_data; bfd_vma * i_data; + /* If the size_t type is smaller than the bfd_size_type, eg because + you are building a 32-bit tool on a 64-bit host, then make sure + that when (number) is cast to (size_t) no information is lost. */ + if (sizeof (size_t) < sizeof (bfd_size_type) + && (bfd_size_type) ((size_t) number) != number) + { + error (_("Size truncation prevents reading %llu elements of size %u\n"), + (unsigned long long) number, ent_size); + return NULL; + } + /* Be kind to memory chekers (eg valgrind, address sanitizer) by not attempting to allocate memory when the read is bound to fail. */ if (ent_size * number > current_file_size) { - error (_("Invalid number of dynamic entries: %lu\n"), - (unsigned long) number); + error (_("Invalid number of dynamic entries: %llu\n"), + (unsigned long long) number); return NULL; } - e_data = (unsigned char *) cmalloc (number, ent_size); + e_data = (unsigned char *) cmalloc ((size_t) number, ent_size); if (e_data == NULL) { - error (_("Out of memory reading %lu dynamic entries\n"), - (unsigned long) number); + error (_("Out of memory reading %llu dynamic entries\n"), + (unsigned long long) number); return NULL; } - if (fread (e_data, ent_size, number, file) != number) + if (fread (e_data, ent_size, (size_t) number, file) != number) { - error (_("Unable to read in %lu bytes of dynamic data\n"), - (unsigned long) (number * ent_size)); + error (_("Unable to read in %llu bytes of dynamic data\n"), + (unsigned long long) (number * ent_size)); free (e_data); return NULL; } - i_data = (bfd_vma *) cmalloc (number, sizeof (*i_data)); + i_data = (bfd_vma *) cmalloc ((size_t) number, sizeof (*i_data)); if (i_data == NULL) { - error (_("Out of memory allocating space for %lu dynamic entries\n"), - (unsigned long) number); + error (_("Out of memory allocating space for %llu dynamic entries\n"), + (unsigned long long) number); free (e_data); return NULL; } @@ -10683,7 +10730,7 @@ process_symbol_table (FILE * file) printf (_(" Length Number %% of total Coverage\n")); for (hn = 0; hn < nbuckets; ++hn) { - for (si = buckets[hn]; si > 0 && si < nchains; si = chains[si]) + for (si = buckets[hn]; si > 0 && si < nchains && si < nbuckets; si = chains[si]) { ++nsyms; if (maxlength < ++lengths[hn]) -- 2.7.4