diff options
author | Nick Clifton <nickc@redhat.com> | 2014-11-12 22:39:58 +0000 |
---|---|---|
committer | Nick Clifton <nickc@redhat.com> | 2014-11-12 22:39:58 +0000 |
commit | f41e4712a7b7ac60f181e7dfc984ca35c222f0d7 (patch) | |
tree | 6ac324979fd61983fb6a27dccf9fe306725789fa /binutils | |
parent | 40e91bc71f7993f2064cec4ffd007f2c814a1b29 (diff) | |
download | fsf-binutils-gdb-f41e4712a7b7ac60f181e7dfc984ca35c222f0d7.zip fsf-binutils-gdb-f41e4712a7b7ac60f181e7dfc984ca35c222f0d7.tar.gz fsf-binutils-gdb-f41e4712a7b7ac60f181e7dfc984ca35c222f0d7.tar.bz2 |
Fix more memory faults uncovered by fuzzing various executables.
PR binutils/17512
* dwarf.c (read_and_display_attr_value): Check that we do not read
past end.
(display_debug_pubnames_worker): Add range checks.
(process_debug_info): Check for invalid pointer sizes.
(display_loc_list): Likewise.
(display_loc_list_dwo): Likewise.
(display_debug_ranges): Likewise.
(display_debug_aranges): Check for invalid address size.
(read_cie): Add range checks. Replace call strchr with while loop.
* objdump.c (dump_dwarf): Replace abort with a warning message.
(print_section_stabs): Improve range checks.
* rdcoff.c (coff_get_slot): Use long for indx parameter type.
Add check for an excesively large index.
* rddbg.c (read_section_stabs_debugging_info): Zero terminate the
string table. Avoid walking off the end of the stabs data.
* stabs.c (parse_stab_string): Add check for a NULL name.
PR binutils/17512
* coffcode.h (coff_slurp_line_table): Set the line number of
corrupt entries to -1.
(coff_slurp_symbol_table): Alway initialise the value of the
symbol.
* coffgen.c (coff_print_symbol): Check that the combined pointer
is valid.
(coff_print_symbol): Do not print negative line numbers.
* peXXigen.c (pe_print_idata): Add range checking displaying
member names.
Diffstat (limited to 'binutils')
-rw-r--r-- | binutils/ChangeLog | 20 | ||||
-rw-r--r-- | binutils/dwarf.c | 144 | ||||
-rw-r--r-- | binutils/objdump.c | 16 | ||||
-rw-r--r-- | binutils/rdcoff.c | 9 | ||||
-rw-r--r-- | binutils/rddbg.c | 40 | ||||
-rw-r--r-- | binutils/stabs.c | 30 |
6 files changed, 191 insertions, 68 deletions
diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 323ae82..3a094f3 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,23 @@ +2014-11-12 Nick Clifton <nickc@redhat.com> + + PR binutils/17512 + * dwarf.c (read_and_display_attr_value): Check that we do not read + past end. + (display_debug_pubnames_worker): Add range checks. + (process_debug_info): Check for invalid pointer sizes. + (display_loc_list): Likewise. + (display_loc_list_dwo): Likewise. + (display_debug_ranges): Likewise. + (display_debug_aranges): Check for invalid address size. + (read_cie): Add range checks. Replace call strchr with while loop. + * objdump.c (dump_dwarf): Replace abort with a warning message. + (print_section_stabs): Improve range checks. + * rdcoff.c (coff_get_slot): Use long for indx parameter type. + Add check for an excesively large index. + * rddbg.c (read_section_stabs_debugging_info): Zero terminate the + string table. Avoid walking off the end of the stabs data. + * stabs.c (parse_stab_string): Add check for a NULL name. + 2014-11-11 Nick Clifton <nickc@redhat.com> PR binutils/17531 diff --git a/binutils/dwarf.c b/binutils/dwarf.c index ee982de..38ea256 100644 --- a/binutils/dwarf.c +++ b/binutils/dwarf.c @@ -399,7 +399,7 @@ process_extended_line_op (unsigned char * data, if (len == 0 || data == end) { - warn (_("badly formed extended line op encountered!\n")); + warn (_("Badly formed extended line op encountered!\n")); return bytes_read; } @@ -1464,9 +1464,9 @@ read_and_display_attr_value (unsigned long attribute, unsigned char * orig_data = data; unsigned int bytes_read; - if (data == end && form != DW_FORM_flag_present) + if (data > end || (data == end && form != DW_FORM_flag_present)) { - warn (_("corrupt attribute\n")); + warn (_("Corrupt attribute\n")); return data; } @@ -1623,6 +1623,12 @@ read_and_display_attr_value (unsigned long attribute, case DW_FORM_exprloc: uvalue = read_uleb128 (data, & bytes_read, end); block_start = data + bytes_read; + /* PR 17512: file: 008-103549-0.001:0.1. */ + if (block_start + uvalue > end) + { + warn (_("Corrupt attribute block length: %lx\n"), (long) uvalue); + uvalue = end - block_start; + } if (do_loc) data = block_start + uvalue; else @@ -1632,6 +1638,11 @@ read_and_display_attr_value (unsigned long attribute, case DW_FORM_block1: SAFE_BYTE_GET (uvalue, data, 1, end); block_start = data + 1; + if (block_start + uvalue > end) + { + warn (_("Corrupt attribute block length: %lx\n"), (long) uvalue); + uvalue = end - block_start; + } if (do_loc) data = block_start + uvalue; else @@ -1641,6 +1652,11 @@ read_and_display_attr_value (unsigned long attribute, case DW_FORM_block2: SAFE_BYTE_GET (uvalue, data, 2, end); block_start = data + 2; + if (block_start + uvalue > end) + { + warn (_("Corrupt attribute block length: %lx\n"), (long) uvalue); + uvalue = end - block_start; + } if (do_loc) data = block_start + uvalue; else @@ -1650,6 +1666,11 @@ read_and_display_attr_value (unsigned long attribute, case DW_FORM_block4: SAFE_BYTE_GET (uvalue, data, 4, end); block_start = data + 4; + if (block_start + uvalue > end) + { + warn (_("Corrupt attribute block length: %lx\n"), (long) uvalue); + uvalue = end - block_start; + } if (do_loc) data = block_start + uvalue; else @@ -2184,7 +2205,7 @@ process_debug_info (struct dwarf_section *section, if (num_units == 0) { - error (_("No comp units in %s section ?"), section->name); + error (_("No comp units in %s section ?\n"), section->name); return 0; } @@ -2193,7 +2214,7 @@ process_debug_info (struct dwarf_section *section, sizeof (* debug_information)); if (debug_information == NULL) { - error (_("Not enough memory for a debug info array of %u entries"), + error (_("Not enough memory for a debug info array of %u entries\n"), num_units); return 0; } @@ -2271,6 +2292,13 @@ process_debug_info (struct dwarf_section *section, } SAFE_BYTE_GET_AND_INC (compunit.cu_pointer_size, hdrptr, 1, end); + /* PR 17512: file: 001-108546-0.001:0.1. */ + if (compunit.cu_pointer_size < 2 || compunit.cu_pointer_size > 8) + { + warn (_("Invalid pointer size (%d) in compunit header, using %d instead\n"), + compunit.cu_pointer_size, offset_size); + compunit.cu_pointer_size = offset_size; + } if (do_types) { @@ -2602,20 +2630,20 @@ read_debug_line_header (struct dwarf_section * section, SAFE_BYTE_GET_AND_INC (linfo->li_length, hdrptr, 4, end); if (linfo->li_length == 0xffffffff) - { - /* This section is 64-bit DWARF 3. */ + { + /* This section is 64-bit DWARF 3. */ SAFE_BYTE_GET_AND_INC (linfo->li_length, hdrptr, 8, end); - offset_size = 8; - initial_length_size = 12; - } - else - { - offset_size = 4; - initial_length_size = 4; - } + offset_size = 8; + initial_length_size = 12; + } + else + { + offset_size = 4; + initial_length_size = 4; + } if (linfo->li_length + initial_length_size > section->size) - { + { /* If the length is just a bias against the initial_length_size then this means that the field has a relocation against it which has not been applied. (Ie we are dealing with an object file, not a linked @@ -2627,11 +2655,10 @@ read_debug_line_header (struct dwarf_section * section, } else { - warn (_("The line info appears to be corrupt - " - "the section is too small\n")); + warn (_("The line info appears to be corrupt - the section is too small\n")); return NULL; } - } + } /* Get and check the version number. */ SAFE_BYTE_GET_AND_INC (linfo->li_version, hdrptr, 2, end); @@ -2639,25 +2666,25 @@ read_debug_line_header (struct dwarf_section * section, if (linfo->li_version != 2 && linfo->li_version != 3 && linfo->li_version != 4) - { - warn (_("Only DWARF version 2, 3 and 4 line info is currently supported.\n")); + { + warn (_("Only DWARF version 2, 3 and 4 line info is currently supported.\n")); return NULL; - } + } SAFE_BYTE_GET_AND_INC (linfo->li_prologue_length, hdrptr, offset_size, end); SAFE_BYTE_GET_AND_INC (linfo->li_min_insn_length, hdrptr, 1, end); if (linfo->li_version >= 4) - { + { SAFE_BYTE_GET_AND_INC (linfo->li_max_ops_per_insn, hdrptr, 1, end); if (linfo->li_max_ops_per_insn == 0) - { - warn (_("Invalid maximum operations per insn.\n")); + { + warn (_("Invalid maximum operations per insn.\n")); return NULL; - } } - else + } + else linfo->li_max_ops_per_insn = 1; SAFE_BYTE_GET_AND_INC (linfo->li_default_is_stmt, hdrptr, 1, end); @@ -3204,7 +3231,7 @@ display_debug_lines_decoded (struct dwarf_section *section, if (ext_op_code_len == 0) { - warn (_("badly formed extended line op encountered!\n")); + warn (_("Badly formed extended line op encountered!\n")); break; } ext_op_code_len += bytes_read; @@ -3598,11 +3625,17 @@ display_debug_pubnames_worker (struct dwarf_section *section, do { + bfd_size_type maxprint; + SAFE_BYTE_GET (offset, data, offset_size, end); if (offset != 0) { data += offset_size; + if (data >= end) + break; + maxprint = (end - data) - 1; + if (is_gnu) { unsigned int kind_data; @@ -3612,6 +3645,7 @@ display_debug_pubnames_worker (struct dwarf_section *section, SAFE_BYTE_GET (kind_data, data, 1, end); data++; + maxprint --; /* GCC computes the kind as the upper byte in the CU index word, and then right shifts it by the CU index size. Left shift KIND to where the gdb-index.h accessor macros @@ -3620,13 +3654,16 @@ display_debug_pubnames_worker (struct dwarf_section *section, kind = GDB_INDEX_SYMBOL_KIND_VALUE (kind_data); kind_name = get_gdb_index_symbol_kind_name (kind); is_static = GDB_INDEX_SYMBOL_STATIC_VALUE (kind_data); - printf (" %-6lx %s,%-10s %s\n", + printf (" %-6lx %s,%-10s %.*s\n", offset, is_static ? _("s") : _("g"), - kind_name, data); + kind_name, (int) maxprint, data); } else - printf (" %-6lx\t%s\n", offset, data); - data += strnlen ((char *) data, end - data) + 1; + printf (" %-6lx\t%.*s\n", offset, (int) maxprint, data); + + data += strnlen ((char *) data, maxprint) + 1; + if (data >= end) + break; } } while (offset != 0); @@ -4135,6 +4172,13 @@ display_loc_list (struct dwarf_section *section, unsigned short length; int need_frame_base; + if (pointer_size < 2 || pointer_size > 8) + { + warn (_("Invalid pointer size (%d) in debug info for entry %d\n"), + pointer_size, debug_info_entry); + return; + } + while (1) { if (start + 2 * pointer_size > section_end) @@ -4247,6 +4291,13 @@ display_loc_list_dwo (struct dwarf_section *section, unsigned int idx; unsigned int bytes_read; + if (pointer_size < 2 || pointer_size > 8) + { + warn (_("Invalid pointer size (%d) in debug info for entry %d\n"), + pointer_size, debug_info_entry); + return; + } + while (1) { printf (" %8.8lx ", offset + (start - *start_ptr)); @@ -4647,7 +4698,8 @@ display_debug_aranges (struct dwarf_section *section, address_size = arange.ar_pointer_size + arange.ar_segment_size; - if (address_size == 0) + /* PR 17512: file: 001-108546-0.001:0.1. */ + if (address_size == 0 || address_size > 8) { error (_("Invalid address size in %s section!\n"), section->name); @@ -4886,11 +4938,18 @@ display_debug_ranges (struct dwarf_section *section, unsigned long base_address; pointer_size = debug_info_p->pointer_size; - offset = range_entry->ranges_offset; next = section_begin + offset; base_address = debug_info_p->base_address; + /* PR 17512: file: 001-101485-0.001:0.1. */ + if (pointer_size < 2 || pointer_size > 8) + { + warn (_("Corrupt pointer size (%d) in debug entry at offset %8.8lx\n"), + pointer_size, offset); + continue; + } + if (dwarf_check != 0 && i > 0) { if (start < next) @@ -5243,6 +5302,10 @@ read_cie (unsigned char *start, unsigned char *end, unsigned char *augmentation_data = NULL; unsigned long augmentation_data_len = 0; + /* PR 17512: file: 001-228113-0.004. */ + if (start >= end) + return end; + fc = (Frame_Chunk *) xmalloc (sizeof (Frame_Chunk)); memset (fc, 0, sizeof (Frame_Chunk)); @@ -5252,7 +5315,16 @@ read_cie (unsigned char *start, unsigned char *end, version = *start++; fc->augmentation = (char *) start; - start = (unsigned char *) strchr ((char *) start, '\0') + 1; + /* PR 17512: file: 001-228113-0.004. + Skip past augmentation name, but avoid running off the end of the data. */ + while (start < end) + if (* start ++ == '\0') + break; + if (start == end) + { + warn (_("No terminator for augmentation name\n")); + return start; + } if (strcmp (fc->augmentation, "eh") == 0) start += eh_addr_size; @@ -6140,7 +6212,7 @@ display_debug_frames (struct dwarf_section *section, if (op >= DW_CFA_lo_user && op <= DW_CFA_hi_user) printf (_(" DW_CFA_??? (User defined call frame op: %#x)\n"), op); else - warn (_("unsupported or unknown Dwarf Call Frame Instruction number: %#x\n"), op); + warn (_("Unsupported or unknown Dwarf Call Frame Instruction number: %#x\n"), op); start = block_end; } } diff --git a/binutils/objdump.c b/binutils/objdump.c index f6c4c16..da68f39 100644 --- a/binutils/objdump.c +++ b/binutils/objdump.c @@ -2388,7 +2388,12 @@ dump_dwarf (bfd *abfd) else if (bfd_little_endian (abfd)) byte_get = byte_get_little_endian; else - abort (); + /* PR 17512: file: objdump-s-endless-loop.tekhex. */ + { + warn (_("File %s does not contain any dwarf debug information\n"), + bfd_get_filename (abfd)); + return; + } switch (bfd_get_arch (abfd)) { @@ -2496,7 +2501,7 @@ print_section_stabs (bfd *abfd, We start the index at -1 because there is a dummy symbol on the front of stabs-in-{coff,elf} sections that supplies sizes. */ - for (i = -1; stabp < stabs_end; stabp += STABSIZE, i++) + for (i = -1; stabp <= stabs_end - STABSIZE; stabp += STABSIZE, i++) { const char *name; unsigned long strx; @@ -2534,10 +2539,13 @@ print_section_stabs (bfd *abfd, } else { + bfd_size_type amt = strx + file_string_table_offset; + /* Using the (possibly updated) string table offset, print the string (if any) associated with this symbol. */ - if ((strx + file_string_table_offset) < stabstr_size) - printf (" %s", &strtab[strx + file_string_table_offset]); + if (amt < stabstr_size) + /* PR 17512: file: 079-79389-0.001:0.1. */ + printf (" %.*s", (int)(stabstr_size - amt), strtab + amt); else printf (" *"); } diff --git a/binutils/rdcoff.c b/binutils/rdcoff.c index 859aefe..6785136 100644 --- a/binutils/rdcoff.c +++ b/binutils/rdcoff.c @@ -83,7 +83,7 @@ struct coff_types debug_type basic[T_MAX + 1]; }; -static debug_type *coff_get_slot (struct coff_types *, int); +static debug_type *coff_get_slot (struct coff_types *, long); static debug_type parse_coff_type (bfd *, struct coff_symbols *, struct coff_types *, long, int, union internal_auxent *, bfd_boolean, void *); @@ -104,12 +104,17 @@ static bfd_boolean external_coff_symbol_p (int sym_class); /* Return the slot for a type. */ static debug_type * -coff_get_slot (struct coff_types *types, int indx) +coff_get_slot (struct coff_types *types, long indx) { struct coff_slots **pps; pps = &types->slots; + /* PR 17512: file: 078-18333-0.001:0.1. + FIXME: The value of 1000 is a guess. Maybe a better heuristic is needed. */ + if (indx / COFF_SLOTS > 1000) + fatal (_("Excessively large slot index: %lx"), indx); + while (indx >= COFF_SLOTS) { if (*pps == NULL) diff --git a/binutils/rddbg.c b/binutils/rddbg.c index bfa54ab..ea8161b 100644 --- a/binutils/rddbg.c +++ b/binutils/rddbg.c @@ -139,7 +139,7 @@ read_section_stabs_debugging_info (bfd *abfd, asymbol **syms, long symcount, } strsize = bfd_section_size (abfd, strsec); - strings = (bfd_byte *) xmalloc (strsize); + strings = (bfd_byte *) xmalloc (strsize + 1); if (! bfd_get_section_contents (abfd, strsec, strings, 0, strsize)) { fprintf (stderr, "%s: %s: %s\n", @@ -147,7 +147,8 @@ read_section_stabs_debugging_info (bfd *abfd, asymbol **syms, long symcount, bfd_errmsg (bfd_get_error ())); return FALSE; } - + /* Zero terminate the strings table, just in case. */ + strings [strsize] = 0; if (shandle == NULL) { shandle = start_stab (dhandle, abfd, TRUE, syms, symcount); @@ -159,7 +160,8 @@ read_section_stabs_debugging_info (bfd *abfd, asymbol **syms, long symcount, stroff = 0; next_stroff = 0; - for (stab = stabs; stab < stabs + stabsize; stab += 12) + /* PR 17512: file: 078-60391-0.001:0.1. */ + for (stab = stabs; stab <= (stabs + stabsize) - 12; stab += 12) { unsigned int strx; int type; @@ -184,33 +186,43 @@ read_section_stabs_debugging_info (bfd *abfd, asymbol **syms, long symcount, } else { + size_t len; char *f, *s; - f = NULL; - - if (stroff + strx > strsize) + if (stroff + strx >= strsize) { - fprintf (stderr, "%s: %s: stab entry %ld is corrupt, strx = 0x%x, type = %d\n", + fprintf (stderr, _("%s: %s: stab entry %ld is corrupt, strx = 0x%x, type = %d\n"), bfd_get_filename (abfd), names[i].secname, (long) (stab - stabs) / 12, strx, type); continue; } s = (char *) strings + stroff + strx; + f = NULL; - while (s[strlen (s) - 1] == '\\' + /* PR 17512: file: 002-87578-0.001:0.1. + It is possible to craft a file where, without the 'strlen (s) > 0', + an attempt to read the byte before 'strings' would occur. */ + while ((len = strlen (s)) > 0 + && s[len - 1] == '\\' && stab + 12 < stabs + stabsize) { char *p; stab += 12; - p = s + strlen (s) - 1; + p = s + len - 1; *p = '\0'; - s = concat (s, - ((char *) strings - + stroff - + bfd_get_32 (abfd, stab)), - (const char *) NULL); + strx = stroff + bfd_get_32 (abfd, stab); + if (strx >= strsize) + { + fprintf (stderr, _("%s: %s: stab entry %ld is corrupt\n"), + bfd_get_filename (abfd), names[i].secname, + (long) (stab - stabs) / 12); + break; + } + else + s = concat (s, (char *) strings + strx, + (const char *) NULL); /* We have to restore the backslash, because, if the linker is hashing stabs strings, we may diff --git a/binutils/stabs.c b/binutils/stabs.c index 2a2674d..33159e9 100644 --- a/binutils/stabs.c +++ b/binutils/stabs.c @@ -836,8 +836,6 @@ parse_stab_string (void *dhandle, struct stab_handle *info, int stabtype, case 'G': { - char leading; - long c; asymbol **ps; /* A global symbol. The value must be extracted from the @@ -846,19 +844,27 @@ parse_stab_string (void *dhandle, struct stab_handle *info, int stabtype, (debug_type **) NULL); if (dtype == DEBUG_TYPE_NULL) return FALSE; - leading = bfd_get_symbol_leading_char (info->abfd); - for (c = info->symcount, ps = info->syms; c > 0; --c, ++ps) + if (name != NULL) { - const char *n; + char leading; + long c; - n = bfd_asymbol_name (*ps); - if (leading != '\0' && *n == leading) - ++n; - if (*n == *name && strcmp (n, name) == 0) - break; + leading = bfd_get_symbol_leading_char (info->abfd); + for (c = info->symcount, ps = info->syms; c > 0; --c, ++ps) + { + const char *n; + + n = bfd_asymbol_name (*ps); + if (leading != '\0' && *n == leading) + ++n; + if (*n == *name && strcmp (n, name) == 0) + break; + } + + if (c > 0) + value = bfd_asymbol_value (*ps); } - if (c > 0) - value = bfd_asymbol_value (*ps); + if (! stab_record_variable (dhandle, info, name, dtype, DEBUG_GLOBAL, value)) return FALSE; |