diff options
author | Nick Clifton <nickc@redhat.com> | 2014-11-03 17:44:00 +0000 |
---|---|---|
committer | Nick Clifton <nickc@redhat.com> | 2014-11-03 17:44:00 +0000 |
commit | 5a3f568b70bdfb91aacdfb66657b56d8c6d242f1 (patch) | |
tree | d11c8695362d773082a427bb3897d598a1ba0998 | |
parent | ba241f2d5a6f1ac57e746d230892e27a70def4a6 (diff) | |
download | gdb-5a3f568b70bdfb91aacdfb66657b56d8c6d242f1.zip gdb-5a3f568b70bdfb91aacdfb66657b56d8c6d242f1.tar.gz gdb-5a3f568b70bdfb91aacdfb66657b56d8c6d242f1.tar.bz2 |
More fixes for buffer overruns instigated by corrupt binaries.
PR binutils/17512
* objdump.c (slurp_symtab): Fail gracefully if the table could not
be read.
(dump_relocs_in_section): Likewise.
* aoutx.h (slurp_symbol_table): Check that computed table size is
not bigger than the file from which is it being read.
(slurp_reloc_table): Likewise.
* coffcode.h (coff_slurp_line_table): Remove unneeded local
'warned'. Do not try to print the details of a symbol with an
invalid index.
* coffgen.c (make_a_sectiobn_from_file): Check computed string
index against length of string table.
(bfd_coff_internal_syment_name): Check read in string offset
against length of string table.
(build_debug_section): Return a pointer to the section used.
(_bfd_coff_read_string_table): Store the length of the string
table in the coff_tdata structure.
(bfd_coff_free_symbols): Set the length of the string table to
zero when it is freed.
(coff_get_normalized_symtab): Check offsets against string table
or data table lengths as appropriate.
* cofflink.c (_bfd_coff_link_input_bfd): Check offset against
length of string table.
* compress.c (bfd_get_full_section_contents): Check computed size
against the size of the file.
* libcoff-in.h (obj_coff_strings_len): Define.
(struct coff_tdata): Add strings_len field.
* libcoff.h: Regenerate.
* peXXigen.c (pe_print_debugdata): Do not attempt to print the
data if the debug section is too small.
* xcofflink.c (xcoff_link_input_bfd): Check offset against
length of string table.
-rw-r--r-- | bfd/ChangeLog | 32 | ||||
-rw-r--r-- | bfd/aoutx.h | 7 | ||||
-rw-r--r-- | bfd/coffcode.h | 11 | ||||
-rw-r--r-- | bfd/coffgen.c | 72 | ||||
-rw-r--r-- | bfd/cofflink.c | 5 | ||||
-rw-r--r-- | bfd/compress.c | 7 | ||||
-rw-r--r-- | bfd/libcoff-in.h | 3 | ||||
-rw-r--r-- | bfd/libcoff.h | 3 | ||||
-rw-r--r-- | bfd/peXXigen.c | 9 | ||||
-rw-r--r-- | bfd/xcofflink.c | 5 | ||||
-rw-r--r-- | binutils/ChangeLog | 7 | ||||
-rw-r--r-- | binutils/objdump.c | 11 |
12 files changed, 140 insertions, 32 deletions
diff --git a/bfd/ChangeLog b/bfd/ChangeLog index d2d8bae..591ff95 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,5 +1,37 @@ 2014-11-03 Nick Clifton <nickc@redhat.com> + PR binutils/17512 + * aoutx.h (slurp_symbol_table): Check that computed table size is + not bigger than the file from which is it being read. + (slurp_reloc_table): Likewise. + * coffcode.h (coff_slurp_line_table): Remove unneeded local + 'warned'. Do not try to print the details of a symbol with an + invalid index. + * coffgen.c (make_a_sectiobn_from_file): Check computed string + index against length of string table. + (bfd_coff_internal_syment_name): Check read in string offset + against length of string table. + (build_debug_section): Return a pointer to the section used. + (_bfd_coff_read_string_table): Store the length of the string + table in the coff_tdata structure. + (bfd_coff_free_symbols): Set the length of the string table to + zero when it is freed. + (coff_get_normalized_symtab): Check offsets against string table + or data table lengths as appropriate. + * cofflink.c (_bfd_coff_link_input_bfd): Check offset against + length of string table. + * compress.c (bfd_get_full_section_contents): Check computed size + against the size of the file. + * libcoff-in.h (obj_coff_strings_len): Define. + (struct coff_tdata): Add strings_len field. + * libcoff.h: Regenerate. + * peXXigen.c (pe_print_debugdata): Do not attempt to print the + data if the debug section is too small. + * xcofflink.c (xcoff_link_input_bfd): Check offset against + length of string table. + +2014-11-03 Nick Clifton <nickc@redhat.com> + * po/fi.po: Updated Finnish translation. 2014-10-31 Andrew Pinski <apinski@cavium.com> diff --git a/bfd/aoutx.h b/bfd/aoutx.h index bef59b4..cb0887a 100644 --- a/bfd/aoutx.h +++ b/bfd/aoutx.h @@ -1756,6 +1756,8 @@ NAME (aout, slurp_symbol_table) (bfd *abfd) return TRUE; /* Nothing to do. */ cached_size *= sizeof (aout_symbol_type); + if (cached_size >= (bfd_size_type) bfd_get_size (abfd)) + return FALSE; cached = (aout_symbol_type *) bfd_zmalloc (cached_size); if (cached == NULL) return FALSE; @@ -2307,6 +2309,11 @@ NAME (aout, slurp_reloc_table) (bfd *abfd, sec_ptr asect, asymbol **symbols) if (reloc_size == 0) return TRUE; /* Nothing to be done. */ + /* PR binutils/17512: Do not even try to + load the relocs if their size is corrupt. */ + if (reloc_size + asect->rel_filepos >= (bfd_size_type) bfd_get_size (abfd)) + return FALSE; + if (bfd_seek (abfd, asect->rel_filepos, SEEK_SET) != 0) return FALSE; diff --git a/bfd/coffcode.h b/bfd/coffcode.h index 6678b88..48709f4 100644 --- a/bfd/coffcode.h +++ b/bfd/coffcode.h @@ -4546,21 +4546,18 @@ coff_slurp_line_table (bfd *abfd, asection *asect) if (cache_ptr->line_number == 0) { - bfd_boolean warned; bfd_signed_vma symndx; coff_symbol_type *sym; nbr_func++; - warned = FALSE; symndx = dst.l_addr.l_symndx; if (symndx < 0 || (bfd_vma) symndx >= obj_raw_syment_count (abfd)) { (*_bfd_error_handler) - (_("%B: warning: illegal symbol index %ld in line numbers"), - abfd, (long) symndx); - symndx = 0; - warned = TRUE; + (_("%B: warning: illegal symbol index 0x%lx in line number entry %d"), + abfd, (long) symndx, counter); + continue; } /* FIXME: We should not be casting between ints and @@ -4569,7 +4566,7 @@ coff_slurp_line_table (bfd *abfd, asection *asect) ((symndx + obj_raw_syments (abfd)) ->u.syment._n._n_n._n_zeroes)); cache_ptr->u.sym = (asymbol *) sym; - if (sym->lineno != NULL && ! warned) + if (sym->lineno != NULL) (*_bfd_error_handler) (_("%B: warning: duplicate line number information for `%s'"), abfd, bfd_asymbol_name (&sym->symbol)); diff --git a/bfd/coffgen.c b/bfd/coffgen.c index f18ddab..d0bf2c1a 100644 --- a/bfd/coffgen.c +++ b/bfd/coffgen.c @@ -84,9 +84,8 @@ make_a_section_from_file (bfd *abfd, strings = _bfd_coff_read_string_table (abfd); if (strings == NULL) return FALSE; - /* FIXME: For extra safety, we should make sure that - strindex does not run us past the end, but right now we - don't know the length of the string table. */ + if ((bfd_size_type)(strindex + 2) >= obj_coff_strings_len (abfd)) + return FALSE; strings += strindex; name = (char *) bfd_alloc (abfd, (bfd_size_type) strlen (strings) + 1 + 1); @@ -464,6 +463,8 @@ _bfd_coff_internal_syment_name (bfd *abfd, if (strings == NULL) return NULL; } + if (sym->_n._n_n._n_offset >= obj_coff_strings_len (abfd)) + return NULL; return strings + sym->_n._n_n._n_offset; } } @@ -1545,7 +1546,7 @@ coff_pointerize_aux (bfd *abfd, we didn't want to go to the trouble until someone needed it. */ static char * -build_debug_section (bfd *abfd) +build_debug_section (bfd *abfd, asection ** sect_return) { char *debug_section; file_ptr position; @@ -1573,6 +1574,8 @@ build_debug_section (bfd *abfd) || bfd_bread (debug_section, sec_size, abfd) != sec_size || bfd_seek (abfd, position, SEEK_SET) != 0) return NULL; + + * sect_return = sect; return debug_section; } @@ -1640,7 +1643,9 @@ _bfd_coff_get_external_symbols (bfd *abfd) /* Read in the external strings. The strings are not loaded until they are needed. This is because we have no simple way of - detecting a missing string table in an archive. */ + detecting a missing string table in an archive. If the strings + are loaded then the STRINGS and STRINGS_LEN fields in the + coff_tdata structure will be set. */ const char * _bfd_coff_read_string_table (bfd *abfd) @@ -1702,6 +1707,7 @@ _bfd_coff_read_string_table (bfd *abfd) } obj_coff_strings (abfd) = strings; + obj_coff_strings_len (abfd) = strsize; return strings; } @@ -1722,6 +1728,7 @@ _bfd_coff_free_symbols (bfd *abfd) { free (obj_coff_strings (abfd)); obj_coff_strings (abfd) = NULL; + obj_coff_strings_len (abfd) = 0; } return TRUE; } @@ -1742,13 +1749,22 @@ coff_get_normalized_symtab (bfd *abfd) char *raw_src; char *raw_end; const char *string_table = NULL; - char *debug_section = NULL; + asection * debug_sec = NULL; + char *debug_sec_data = NULL; bfd_size_type size; if (obj_raw_syments (abfd) != NULL) return obj_raw_syments (abfd); - size = obj_raw_syment_count (abfd) * sizeof (combined_entry_type); + size = obj_raw_syment_count (abfd); + if (size == 0) + return NULL; + /* PR binutils/17512: Do not even try to load + a symbol table bigger than the entire file... */ + if (size >= (bfd_size_type) bfd_get_size (abfd)) + return NULL; + + size *= sizeof (combined_entry_type); internal = (combined_entry_type *) bfd_zalloc (abfd, size); if (internal == NULL && size != 0) return NULL; @@ -1819,11 +1835,14 @@ coff_get_normalized_symtab (bfd *abfd) if (string_table == NULL) return NULL; } - - internal_ptr->u.syment._n._n_n._n_offset = - ((bfd_hostptr_t) - (string_table - + (internal_ptr + 1)->u.auxent.x_file.x_n.x_offset)); + if ((bfd_size_type)((internal_ptr + 1)->u.auxent.x_file.x_n.x_offset) + >= obj_coff_strings_len (abfd)) + internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) _("<corrupt>"); + else + internal_ptr->u.syment._n._n_n._n_offset = + ((bfd_hostptr_t) + (string_table + + (internal_ptr + 1)->u.auxent.x_file.x_n.x_offset)); } else { @@ -1878,18 +1897,31 @@ coff_get_normalized_symtab (bfd *abfd) if (string_table == NULL) return NULL; } - internal_ptr->u.syment._n._n_n._n_offset = - ((bfd_hostptr_t) - (string_table - + internal_ptr->u.syment._n._n_n._n_offset)); + if (internal_ptr->u.syment._n._n_n._n_offset >= obj_coff_strings_len (abfd)) + internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) _("<corrupt>"); + else + internal_ptr->u.syment._n._n_n._n_offset = + ((bfd_hostptr_t) + (string_table + + internal_ptr->u.syment._n._n_n._n_offset)); } else { /* Long name in debug section. Very similar. */ - if (debug_section == NULL) - debug_section = build_debug_section (abfd); - internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) - (debug_section + internal_ptr->u.syment._n._n_n._n_offset); + if (debug_sec_data == NULL) + debug_sec_data = build_debug_section (abfd, & debug_sec); + if (debug_sec_data != NULL) + { + BFD_ASSERT (debug_sec != NULL); + /* PR binutils/17512: Catch out of range offsets into the debug data. */ + if (internal_ptr->u.syment._n._n_n._n_offset > debug_sec->size) + internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) _("<corrupt>"); + else + internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) + (debug_sec_data + internal_ptr->u.syment._n._n_n._n_offset); + } + else + internal_ptr->u.syment._n._n_n._n_offset = (bfd_hostptr_t) ""; } } internal_ptr += internal_ptr->u.syment.n_numaux; diff --git a/bfd/cofflink.c b/bfd/cofflink.c index 8c3f71b..2782795 100644 --- a/bfd/cofflink.c +++ b/bfd/cofflink.c @@ -2003,7 +2003,10 @@ _bfd_coff_link_input_bfd (struct coff_final_link_info *flaginfo, bfd *input_bfd) if (strings == NULL) return FALSE; } - filename = strings + auxp->x_file.x_n.x_offset; + if ((bfd_size_type) auxp->x_file.x_n.x_offset >= obj_coff_strings_len (input_bfd)) + filename = _("<corrupt>"); + else + filename = strings + auxp->x_file.x_n.x_offset; indx = _bfd_stringtab_add (flaginfo->strtab, filename, hash, copy); if (indx == (bfd_size_type) -1) diff --git a/bfd/compress.c b/bfd/compress.c index 20eef95..083a7df 100644 --- a/bfd/compress.c +++ b/bfd/compress.c @@ -177,6 +177,13 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr) switch (sec->compress_status) { case COMPRESS_SECTION_NONE: + /* PR binutils/17512: Avoid malloc or file reading errors due to + ridiculous section sizes. But ignore linker created objects + with no contents (yet). */ + if (bfd_get_size (abfd) > 0 + && sz > (bfd_size_type) bfd_get_size (abfd)) + return FALSE; + if (p == NULL) { p = (bfd_byte *) bfd_malloc (sz); diff --git a/bfd/libcoff-in.h b/bfd/libcoff-in.h index 6162f2e..6b6eb28 100644 --- a/bfd/libcoff-in.h +++ b/bfd/libcoff-in.h @@ -35,6 +35,7 @@ #define obj_coff_external_syms(bfd) (coff_data (bfd)->external_syms) #define obj_coff_keep_syms(bfd) (coff_data (bfd)->keep_syms) #define obj_coff_strings(bfd) (coff_data (bfd)->strings) +#define obj_coff_strings_len(bfd) (coff_data (bfd)->strings_len) #define obj_coff_keep_strings(bfd) (coff_data (bfd)->keep_strings) #define obj_coff_sym_hashes(bfd) (coff_data (bfd)->sym_hashes) #define obj_coff_strings_written(bfd) (coff_data (bfd)->strings_written) @@ -75,6 +76,8 @@ typedef struct coff_tdata /* The string table. May be NULL. Read by _bfd_coff_read_string_table. */ char *strings; + /* The length of the strings table. For error checking. */ + bfd_size_type strings_len; /* If this is TRUE, the strings may not be freed. */ bfd_boolean keep_strings; /* If this is TRUE, the strings have been written out already. */ diff --git a/bfd/libcoff.h b/bfd/libcoff.h index 12f19d0..7ed52de 100644 --- a/bfd/libcoff.h +++ b/bfd/libcoff.h @@ -39,6 +39,7 @@ #define obj_coff_external_syms(bfd) (coff_data (bfd)->external_syms) #define obj_coff_keep_syms(bfd) (coff_data (bfd)->keep_syms) #define obj_coff_strings(bfd) (coff_data (bfd)->strings) +#define obj_coff_strings_len(bfd) (coff_data (bfd)->strings_len) #define obj_coff_keep_strings(bfd) (coff_data (bfd)->keep_strings) #define obj_coff_sym_hashes(bfd) (coff_data (bfd)->sym_hashes) #define obj_coff_strings_written(bfd) (coff_data (bfd)->strings_written) @@ -79,6 +80,8 @@ typedef struct coff_tdata /* The string table. May be NULL. Read by _bfd_coff_read_string_table. */ char *strings; + /* The length of the strings table. For error checking. */ + bfd_size_type strings_len; /* If this is TRUE, the strings may not be freed. */ bfd_boolean keep_strings; /* If this is TRUE, the strings have been written out already. */ diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index 1a5cb31..f1a1189 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -2514,6 +2514,13 @@ pe_print_debugdata (bfd * abfd, void * vfile) section->name); return TRUE; } + else if (section->size < size) + { + fprintf (file, + _("\nError: section %s contains the debug data starting address but it is too small\n"), + section->name); + return FALSE; + } fprintf (file, _("\nThere is a debug directory in %s at 0x%lx\n\n"), section->name, (unsigned long) addr); @@ -2523,7 +2530,7 @@ pe_print_debugdata (bfd * abfd, void * vfile) fprintf (file, _("Type Size Rva Offset\n")); - /* Read the whole section. */ + /* Read the whole section. */ if (!bfd_malloc_and_get_section (abfd, section, &data)) { if (data != NULL) diff --git a/bfd/xcofflink.c b/bfd/xcofflink.c index 1ca9c2e..9522974 100644 --- a/bfd/xcofflink.c +++ b/bfd/xcofflink.c @@ -4494,7 +4494,10 @@ xcoff_link_input_bfd (struct xcoff_final_link_info *flinfo, if (strings == NULL) return FALSE; } - filename = strings + aux.x_file.x_n.x_offset; + if ((bfd_size_type) aux.x_file.x_n.x_offset >= obj_coff_strings_len (input_bfd)) + filename = _("<corrupt>"); + else + filename = strings + aux.x_file.x_n.x_offset; indx = _bfd_stringtab_add (flinfo->strtab, filename, hash, copy); if (indx == (bfd_size_type) -1) diff --git a/binutils/ChangeLog b/binutils/ChangeLog index cb5ec95..1f551dd 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,5 +1,12 @@ 2014-11-03 Nick Clifton <nickc@redhat.com> + PR binutils/17512 + * objdump.c (slurp_symtab): Fail gracefully if the table could not + be read. + (dump_relocs_in_section): Likewise. + +2014-11-03 Nick Clifton <nickc@redhat.com> + * po/fi.po: Updated Finnish translation. * po/sv.po: Updated Swedish translation. diff --git a/binutils/objdump.c b/binutils/objdump.c index 413de56..f6c4c16 100644 --- a/binutils/objdump.c +++ b/binutils/objdump.c @@ -562,7 +562,10 @@ slurp_symtab (bfd *abfd) storage = bfd_get_symtab_upper_bound (abfd); if (storage < 0) - bfd_fatal (bfd_get_filename (abfd)); + { + non_fatal (_("failed to read symbol table from: %s"), bfd_get_filename (abfd)); + bfd_fatal (_("error message was")); + } if (storage) sy = (asymbol **) xmalloc (storage); @@ -3108,7 +3111,11 @@ dump_relocs_in_section (bfd *abfd, relcount = bfd_canonicalize_reloc (abfd, section, relpp, syms); if (relcount < 0) - bfd_fatal (bfd_get_filename (abfd)); + { + printf ("\n"); + non_fatal (_("failed to read relocs in: %s"), bfd_get_filename (abfd)); + bfd_fatal (_("error message was")); + } else if (relcount == 0) printf (" (none)\n\n"); else |