From b2f93c5011cab00f31669363577b938697752e43 Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Tue, 28 Oct 2014 10:50:17 +0000 Subject: [PATCH] Import patches from the master branch which prevent seg-faults when parsing corrupt binaries. 2014-10-28 Andreas Schwab Nick Clifton PR binutils/17510 * srec.c (srec_bad_byte): Increase size of buf to allow for negative values. (srec_scan): Use an unsigned char buffer to hold header bytes. 2014-10-27 Nick Clifton PR binutils/17512 * elf.c (bfd_section_from_shdr): Detect and warn about ELF binaries with a group of sections linked by the string table indicies. * peXXigen.c (_bfd_XXi_swap_aouthdr_in): Handle corrupt binaries with an invalid value for NumberOfRvaAndSizes. (pe_print_edata): Detect out of range rvas and entry counts for the Export Address table, Name Pointer table and Ordinal table. PR binutils/17510 * elf.c (setup_group): Improve handling of corrupt group sections. --- bfd/ChangeLog | 25 +++++++ bfd/elf.c | 226 ++++++++++++++++++++++++++++++++++++++++----------------- bfd/peXXigen.c | 29 +++++++- bfd/srec.c | 4 +- 4 files changed, 212 insertions(+), 72 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 518e77a..19b8bb4 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,28 @@ +2014-10-28 Nick Clifton + + * Apply trunk patches: + + 2014-10-28 Andreas Schwab + Nick Clifton + PR binutils/17510 + * srec.c (srec_bad_byte): Increase size of buf to allow for + negative values. + (srec_scan): Use an unsigned char buffer to hold header bytes. + + 2014-10-27 Nick Clifton + PR binutils/17512 + * elf.c (bfd_section_from_shdr): Detect and warn about ELF + binaries with a group of sections linked by the string table + indicies. + * peXXigen.c (_bfd_XXi_swap_aouthdr_in): Handle corrupt binaries + with an invalid value for NumberOfRvaAndSizes. + (pe_print_edata): Detect out of range rvas and entry counts for + the Export Address table, Name Pointer table and Ordinal table. + + PR binutils/17510 + * elf.c (setup_group): Improve handling of corrupt group + sections. + 2014-10-28 Alan Modra Apply trunk patches diff --git a/bfd/elf.c b/bfd/elf.c index c884d1d..949221f 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -608,9 +608,10 @@ setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect) if (shdr->contents == NULL) { _bfd_error_handler - (_("%B: Corrupt size field in group section header: 0x%lx"), abfd, shdr->sh_size); + (_("%B: corrupt size field in group section header: 0x%lx"), abfd, shdr->sh_size); bfd_set_error (bfd_error_bad_value); - return FALSE; + -- num_group; + continue; } memset (shdr->contents, 0, amt); @@ -618,7 +619,16 @@ setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect) if (bfd_seek (abfd, shdr->sh_offset, SEEK_SET) != 0 || (bfd_bread (shdr->contents, shdr->sh_size, abfd) != shdr->sh_size)) - return FALSE; + { + _bfd_error_handler + (_("%B: invalid size field in group section header: 0x%lx"), abfd, shdr->sh_size); + bfd_set_error (bfd_error_bad_value); + -- num_group; + /* PR 17510: If the group contents are even partially + corrupt, do not allow any of the contents to be used. */ + memset (shdr->contents, 0, amt); + continue; + } /* Translate raw contents, a flag word followed by an array of elf section indices all in target byte order, @@ -651,6 +661,21 @@ setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect) } } } + + /* PR 17510: Corrupt binaries might contain invalid groups. */ + if (num_group != (unsigned) elf_tdata (abfd)->num_group) + { + elf_tdata (abfd)->num_group = num_group; + + /* If all groups are invalid then fail. */ + if (num_group == 0) + { + elf_tdata (abfd)->group_sect_ptr = NULL; + elf_tdata (abfd)->num_group = num_group = -1; + (*_bfd_error_handler) (_("%B: no valid group sections found"), abfd); + bfd_set_error (bfd_error_bad_value); + } + } } } @@ -716,6 +741,7 @@ setup_group (bfd *abfd, Elf_Internal_Shdr *hdr, asection *newsect) { (*_bfd_error_handler) (_("%B: no group info for section %A"), abfd, newsect); + return FALSE; } return TRUE; } @@ -1552,38 +1578,67 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) Elf_Internal_Ehdr *ehdr; const struct elf_backend_data *bed; const char *name; + bfd_boolean ret = TRUE; + static bfd_boolean * sections_being_created = NULL; + static unsigned int nesting = 0; if (shindex >= elf_numsections (abfd)) return FALSE; + if (++ nesting > 3) + { + /* PR17512: A corrupt ELF binary might contain a recursive group of + sections, each the string indicies pointing to the next in the + loop. Detect this here, by refusing to load a section that we are + already in the process of loading. We only trigger this test if + we have nested at least three sections deep as normal ELF binaries + can expect to recurse at least once. */ + + if (sections_being_created == NULL) + { + /* FIXME: It would be more efficient to attach this array to the bfd somehow. */ + sections_being_created = (bfd_boolean *) + bfd_zalloc (abfd, elf_numsections (abfd) * sizeof (bfd_boolean)); + } + if (sections_being_created [shindex]) + { + (*_bfd_error_handler) + (_("%B: warning: loop in section dependencies detected"), abfd); + return FALSE; + } + sections_being_created [shindex] = TRUE; + } + hdr = elf_elfsections (abfd)[shindex]; ehdr = elf_elfheader (abfd); name = bfd_elf_string_from_elf_section (abfd, ehdr->e_shstrndx, hdr->sh_name); if (name == NULL) - return FALSE; + goto fail; bed = get_elf_backend_data (abfd); switch (hdr->sh_type) { case SHT_NULL: /* Inactive section. Throw it away. */ - return TRUE; + goto success; - case SHT_PROGBITS: /* Normal section with contents. */ - case SHT_NOBITS: /* .bss section. */ - case SHT_HASH: /* .hash section. */ - case SHT_NOTE: /* .note section. */ + case SHT_PROGBITS: /* Normal section with contents. */ + case SHT_NOBITS: /* .bss section. */ + case SHT_HASH: /* .hash section. */ + case SHT_NOTE: /* .note section. */ case SHT_INIT_ARRAY: /* .init_array section. */ case SHT_FINI_ARRAY: /* .fini_array section. */ case SHT_PREINIT_ARRAY: /* .preinit_array section. */ case SHT_GNU_LIBLIST: /* .gnu.liblist section. */ case SHT_GNU_HASH: /* .gnu.hash section. */ - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + goto success; case SHT_DYNAMIC: /* Dynamic linking information. */ if (! _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex)) - return FALSE; + goto fail; + if (hdr->sh_link > elf_numsections (abfd)) { /* PR 10478: Accept Solaris binaries with a sh_link @@ -1597,11 +1652,11 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) break; /* Otherwise fall through. */ default: - return FALSE; + goto fail; } } else if (elf_elfsections (abfd)[hdr->sh_link] == NULL) - return FALSE; + goto fail; else if (elf_elfsections (abfd)[hdr->sh_link]->sh_type != SHT_STRTAB) { Elf_Internal_Shdr *dynsymhdr; @@ -1630,24 +1685,26 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) } } } - break; + goto success; - case SHT_SYMTAB: /* A symbol table */ + case SHT_SYMTAB: /* A symbol table. */ if (elf_onesymtab (abfd) == shindex) - return TRUE; + goto success; if (hdr->sh_entsize != bed->s->sizeof_sym) - return FALSE; + goto fail; + if (hdr->sh_info * hdr->sh_entsize > hdr->sh_size) { if (hdr->sh_size != 0) - return FALSE; + goto fail; /* Some assemblers erroneously set sh_info to one with a zero sh_size. ld sees this as a global symbol count of (unsigned) -1. Fix it here. */ hdr->sh_info = 0; - return TRUE; + goto success; } + BFD_ASSERT (elf_onesymtab (abfd) == 0); elf_onesymtab (abfd) = shindex; elf_tdata (abfd)->symtab_hdr = *hdr; @@ -1664,7 +1721,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) && (abfd->flags & DYNAMIC) != 0 && ! _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex)) - return FALSE; + goto fail; /* Go looking for SHT_SYMTAB_SHNDX too, since if there is one we can't read symbols without that section loaded as well. It @@ -1690,26 +1747,29 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) break; } if (i != shindex) - return bfd_section_from_shdr (abfd, i); + ret = bfd_section_from_shdr (abfd, i); } - return TRUE; + goto success; - case SHT_DYNSYM: /* A dynamic symbol table */ + case SHT_DYNSYM: /* A dynamic symbol table. */ if (elf_dynsymtab (abfd) == shindex) - return TRUE; + goto success; if (hdr->sh_entsize != bed->s->sizeof_sym) - return FALSE; + goto fail; + if (hdr->sh_info * hdr->sh_entsize > hdr->sh_size) { if (hdr->sh_size != 0) - return FALSE; + goto fail; + /* Some linkers erroneously set sh_info to one with a zero sh_size. ld sees this as a global symbol count of (unsigned) -1. Fix it here. */ hdr->sh_info = 0; - return TRUE; + goto success; } + BFD_ASSERT (elf_dynsymtab (abfd) == 0); elf_dynsymtab (abfd) = shindex; elf_tdata (abfd)->dynsymtab_hdr = *hdr; @@ -1718,34 +1778,38 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) /* Besides being a symbol table, we also treat this as a regular section, so that objcopy can handle it. */ - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + goto success; - case SHT_SYMTAB_SHNDX: /* Symbol section indices when >64k sections */ + case SHT_SYMTAB_SHNDX: /* Symbol section indices when >64k sections. */ if (elf_symtab_shndx (abfd) == shindex) - return TRUE; + goto success; BFD_ASSERT (elf_symtab_shndx (abfd) == 0); elf_symtab_shndx (abfd) = shindex; elf_tdata (abfd)->symtab_shndx_hdr = *hdr; elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->symtab_shndx_hdr; - return TRUE; + goto success; - case SHT_STRTAB: /* A string table */ + case SHT_STRTAB: /* A string table. */ if (hdr->bfd_section != NULL) - return TRUE; + goto success; + if (ehdr->e_shstrndx == shindex) { elf_tdata (abfd)->shstrtab_hdr = *hdr; elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->shstrtab_hdr; - return TRUE; + goto success; } + if (elf_elfsections (abfd)[elf_onesymtab (abfd)]->sh_link == shindex) { symtab_strtab: elf_tdata (abfd)->strtab_hdr = *hdr; elf_elfsections (abfd)[shindex] = &elf_tdata (abfd)->strtab_hdr; - return TRUE; + goto success; } + if (elf_elfsections (abfd)[elf_dynsymtab (abfd)]->sh_link == shindex) { dynsymtab_strtab: @@ -1754,8 +1818,9 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) elf_elfsections (abfd)[shindex] = hdr; /* We also treat this as a regular section, so that objcopy can handle it. */ - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, - shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, + shindex); + goto success; } /* If the string table isn't one of the above, then treat it as a @@ -1773,9 +1838,9 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) { /* Prevent endless recursion on broken objects. */ if (i == shindex) - return FALSE; + goto fail; if (! bfd_section_from_shdr (abfd, i)) - return FALSE; + goto fail; if (elf_onesymtab (abfd) == i) goto symtab_strtab; if (elf_dynsymtab (abfd) == i) @@ -1783,7 +1848,8 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) } } } - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + goto success; case SHT_REL: case SHT_RELA: @@ -1798,7 +1864,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) if (hdr->sh_entsize != (bfd_size_type) (hdr->sh_type == SHT_REL ? bed->s->sizeof_rel : bed->s->sizeof_rela)) - return FALSE; + goto fail; /* Check for a bogus link to avoid crashing. */ if (hdr->sh_link >= num_sec) @@ -1806,8 +1872,9 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) ((*_bfd_error_handler) (_("%B: invalid link %lu for reloc section %s (index %u)"), abfd, hdr->sh_link, name, shindex)); - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, - shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, + shindex); + goto success; } /* For some incomprehensible reason Oracle distributes @@ -1848,7 +1915,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) if ((elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_SYMTAB || elf_elfsections (abfd)[hdr->sh_link]->sh_type == SHT_DYNSYM) && ! bfd_section_from_shdr (abfd, hdr->sh_link)) - return FALSE; + goto fail; /* If this reloc section does not use the main symbol table we don't treat it as a reloc section. BFD can't adequately @@ -1863,14 +1930,18 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) || hdr->sh_info >= num_sec || elf_elfsections (abfd)[hdr->sh_info]->sh_type == SHT_REL || elf_elfsections (abfd)[hdr->sh_info]->sh_type == SHT_RELA) - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, - shindex); + { + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, + shindex); + goto success; + } if (! bfd_section_from_shdr (abfd, hdr->sh_info)) - return FALSE; + goto fail; + target_sect = bfd_section_from_elf_index (abfd, hdr->sh_info); if (target_sect == NULL) - return FALSE; + goto fail; esdt = elf_section_data (target_sect); if (hdr->sh_type == SHT_RELA) @@ -1882,7 +1953,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) amt = sizeof (*hdr2); hdr2 = (Elf_Internal_Shdr *) bfd_alloc (abfd, amt); if (hdr2 == NULL) - return FALSE; + goto fail; *hdr2 = *hdr; *p_hdr = hdr2; elf_elfsections (abfd)[shindex] = hdr2; @@ -1898,34 +1969,40 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) target_sect->use_rela_p = 1; } abfd->flags |= HAS_RELOC; - return TRUE; + goto success; } case SHT_GNU_verdef: elf_dynverdef (abfd) = shindex; elf_tdata (abfd)->dynverdef_hdr = *hdr; - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + goto success; case SHT_GNU_versym: if (hdr->sh_entsize != sizeof (Elf_External_Versym)) - return FALSE; + goto fail; + elf_dynversym (abfd) = shindex; elf_tdata (abfd)->dynversym_hdr = *hdr; - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + goto success; case SHT_GNU_verneed: elf_dynverref (abfd) = shindex; elf_tdata (abfd)->dynverref_hdr = *hdr; - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + goto success; case SHT_SHLIB: - return TRUE; + goto success; case SHT_GROUP: if (! IS_VALID_GROUP_SECTION_HEADER (hdr, GRP_ENTRY_SIZE)) - return FALSE; + goto fail; + if (!_bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex)) - return FALSE; + goto fail; + if (hdr->contents != NULL) { Elf_Internal_Group *idx = (Elf_Internal_Group *) hdr->contents; @@ -1951,7 +2028,7 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) } } } - break; + goto success; default: /* Possibly an attributes section. */ @@ -1959,14 +2036,14 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) || hdr->sh_type == bed->obj_attrs_section_type) { if (! _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex)) - return FALSE; + goto fail; _bfd_elf_parse_attributes (abfd, hdr); - return TRUE; + goto success; } /* Check for any processor-specific section types. */ if (bed->elf_backend_section_from_shdr (abfd, hdr, name, shindex)) - return TRUE; + goto success; if (hdr->sh_type >= SHT_LOUSER && hdr->sh_type <= SHT_HIUSER) { @@ -1978,9 +2055,12 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) "specific section `%s' [0x%8x]"), abfd, name, hdr->sh_type); else - /* Allow sections reserved for applications. */ - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, - shindex); + { + /* Allow sections reserved for applications. */ + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, + shindex); + goto success; + } } else if (hdr->sh_type >= SHT_LOPROC && hdr->sh_type <= SHT_HIPROC) @@ -2001,8 +2081,11 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) "`%s' [0x%8x]"), abfd, name, hdr->sh_type); else - /* Otherwise it should be processed. */ - return _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + { + /* Otherwise it should be processed. */ + ret = _bfd_elf_make_section_from_shdr (abfd, hdr, name, shindex); + goto success; + } } else /* FIXME: We should handle this section. */ @@ -2010,10 +2093,17 @@ bfd_section_from_shdr (bfd *abfd, unsigned int shindex) (_("%B: don't know how to handle section `%s' [0x%8x]"), abfd, name, hdr->sh_type); - return FALSE; + goto fail; } - return TRUE; + fail: + ret = FALSE; + success: + if (sections_being_created) + sections_being_created [shindex] = FALSE; + if (-- nesting == 0) + sections_being_created = NULL; + return ret; } /* Return the local symbol specified by ABFD, R_SYMNDX. */ diff --git a/bfd/peXXigen.c b/bfd/peXXigen.c index 2fb631c..6129085 100644 --- a/bfd/peXXigen.c +++ b/bfd/peXXigen.c @@ -504,6 +504,17 @@ _bfd_XXi_swap_aouthdr_in (bfd * abfd, { int idx; + /* PR 17512: Corrupt PE binaries can cause seg-faults. */ + if (a->NumberOfRvaAndSizes > 16) + { + (*_bfd_error_handler) + (_("%B: aout header specifies an invalid number of data-directory entries: %d"), + abfd, a->NumberOfRvaAndSizes); + /* Paranoia: If the number is corrupt, then assume that the + actual entries themselves might be corrupt as well. */ + a->NumberOfRvaAndSizes = 0; + } + for (idx = 0; idx < a->NumberOfRvaAndSizes; idx++) { /* If data directory is empty, rva also should be 0. */ @@ -1693,7 +1704,12 @@ pe_print_edata (bfd * abfd, void * vfile) _("\nExport Address Table -- Ordinal Base %ld\n"), edt.base); - for (i = 0; i < edt.num_functions; ++i) + /* PR 17512: Handle corrupt PE binaries. */ + if (edt.eat_addr + (edt.num_functions * 4) - adj >= datasize) + fprintf (file, _("\tInvalid Export Address Table rva (0x%lx) or entry count (0x%lx)\n"), + (long) edt.eat_addr, + (long) edt.num_functions); + else for (i = 0; i < edt.num_functions; ++i) { bfd_vma eat_member = bfd_get_32 (abfd, data + edt.eat_addr + (i * 4) - adj); @@ -1729,7 +1745,16 @@ pe_print_edata (bfd * abfd, void * vfile) fprintf (file, _("\n[Ordinal/Name Pointer] Table\n")); - for (i = 0; i < edt.num_names; ++i) + /* PR 17512: Handle corrupt PE binaries. */ + if (edt.npt_addr + (edt.num_names * 4) - adj >= datasize) + fprintf (file, _("\tInvalid Name Pointer Table rva (0x%lx) or entry count (0x%lx)\n"), + (long) edt.npt_addr, + (long) edt.num_names); + else if (edt.ot_addr + (edt.num_names * 2) - adj >= datasize) + fprintf (file, _("\tInvalid Ordinal Table rva (0x%lx) or entry count (0x%lx)\n"), + (long) edt.ot_addr, + (long) edt.num_names); + else for (i = 0; i < edt.num_names; ++i) { bfd_vma name_ptr = bfd_get_32 (abfd, data + diff --git a/bfd/srec.c b/bfd/srec.c index 9ed2080..5f9a546 100644 --- a/bfd/srec.c +++ b/bfd/srec.c @@ -246,7 +246,7 @@ srec_bad_byte (bfd *abfd, } else { - char buf[10]; + char buf[40]; if (! ISPRINT (c)) sprintf (buf, "\\%03o", (unsigned int) c); @@ -452,7 +452,7 @@ srec_scan (bfd *abfd) case 'S': { file_ptr pos; - char hdr[3]; + unsigned char hdr[3]; unsigned int bytes, min_bytes; bfd_vma address; bfd_byte *data; -- 2.7.4