From c9c1d674406c5fff9d2f2ea771e4288cb6bf4e5a Mon Sep 17 00:00:00 2001 From: Espen Grindhaug Date: Thu, 27 Nov 2014 15:49:23 +0000 Subject: [PATCH] Fixes an infinite loop in readelf parsing a corrupt binary, and other minor corrections. PR binutils/17531 * readelf.c (get_data): Move excessive length check to earlier on in the function and allow for wraparound in the arithmetic. (get_32bit_elf_symbols): Terminate early if the section size is zero. Check for an invalid sh_entsize. Check for an index section with an invalid size. (get_64bit_elf_symbols): Likewise. (process_section_groups): Check for an invalid sh_entsize. --- binutils/ChangeLog | 12 ++++++ binutils/readelf.c | 96 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 84 insertions(+), 24 deletions(-) diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 131925a319..709c1cc605 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,15 @@ +2014-11-27 Espen Grindhaug + Nick Clifton + + PR binutils/17531 + * readelf.c (get_data): Move excessive length check to earlier on + in the function and allow for wraparound in the arithmetic. + (get_32bit_elf_symbols): Terminate early if the section size is + zero. Check for an invalid sh_entsize. Check for an index + section with an invalid size. + (get_64bit_elf_symbols): Likewise. + (process_section_groups): Check for an invalid sh_entsize. + 2014-11-24 Mark Wielaard * dwarf.c (read_and_display_attr_value): Handle DW_LANG_C11, diff --git a/binutils/readelf.c b/binutils/readelf.c index 71fc827f28..c00b62b908 100644 --- a/binutils/readelf.c +++ b/binutils/readelf.c @@ -165,7 +165,7 @@ #endif char * program_name = "readelf"; -static long archive_file_offset; +static unsigned long archive_file_offset; static unsigned long archive_file_size; static bfd_size_type current_file_size; static unsigned long dynamic_addr; @@ -313,21 +313,35 @@ static const char *get_symbol_version_string } \ while (0) -/* Retrieve NMEMB structures, each SIZE bytes long from FILE starting at OFFSET. +/* Retrieve NMEMB structures, each SIZE bytes long from FILE starting at OFFSET + + the offset of the current archive member, if we are examining an archive. Put the retrieved data into VAR, if it is not NULL. Otherwise allocate a buffer using malloc and fill that. In either case return the pointer to the start of the retrieved data or NULL if something went wrong. If something does go wrong - emit an error message using REASON as part of the context. */ + and REASON is not NULL then emit an error message using REASON as part of the + context. */ static void * -get_data (void * var, FILE * file, long offset, size_t size, size_t nmemb, +get_data (void * var, FILE * file, unsigned long offset, size_t size, size_t nmemb, const char * reason) { void * mvar; + size_t amt = size * nmemb; if (size == 0 || nmemb == 0) 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); + return NULL; + } + if (fseek (file, archive_file_offset + offset, SEEK_SET)) { if (reason) @@ -336,16 +350,6 @@ get_data (void * var, FILE * file, long offset, size_t size, size_t nmemb, 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 (offset + archive_file_offset + size * nmemb > current_file_size) - { - if (reason) - error (_("Reading 0x%lx bytes extends past end of file for %s\n"), - (unsigned long) (size * nmemb), reason); - return NULL; - } - mvar = var; if (mvar == NULL) { @@ -362,14 +366,14 @@ get_data (void * var, FILE * file, long offset, size_t size, size_t nmemb, return NULL; } - ((char *) mvar)[size * nmemb] = '\0'; + ((char *) mvar)[amt] = '\0'; } if (fread (mvar, size, nmemb, file) != nmemb) { if (reason) error (_("Unable to read in 0x%lx bytes of %s\n"), - (unsigned long)(size * nmemb), reason); + (unsigned long) amt, reason); if (mvar != var) free (mvar); return NULL; @@ -4756,10 +4760,18 @@ get_32bit_elf_symbols (FILE * file, Elf_Internal_Sym * psym; unsigned int j; - /* Run some sanity checks first. */ - if (section->sh_entsize == 0) + if (section->sh_size == 0) { - error (_("sh_entsize is zero\n")); + if (num_syms_return != NULL) + * num_syms_return = 0; + return NULL; + } + + /* Run some sanity checks first. */ + if (section->sh_entsize == 0 || section->sh_entsize > section->sh_size) + { + error (_("Section %s has an invalid sh_entsize of 0x%lx\n"), + printable_section_name (section), (unsigned long) section->sh_entsize); goto exit_point; } @@ -4774,7 +4786,8 @@ get_32bit_elf_symbols (FILE * file, if (number * sizeof (Elf32_External_Sym) > section->sh_size + 1) { - error (_("Invalid sh_entsize\n")); + error (_("Size (0x%lx) of section %s is not a multiple of its sh_entsize (0x%lx)\n"), + section->sh_size, printable_section_name (section), section->sh_entsize); goto exit_point; } @@ -4794,6 +4807,15 @@ get_32bit_elf_symbols (FILE * file, _("symbol table section indicies")); if (shndx == NULL) goto exit_point; + /* PR17531: file: heap-buffer-overflow */ + else if (symtab_shndx_hdr->sh_size / sizeof (Elf_External_Sym_Shndx) < number) + { + error (_("Index section %s has an sh_size of 0x%lx - expected 0x%lx\n"), + printable_section_name (symtab_shndx_hdr), + (unsigned long) symtab_shndx_hdr->sh_size, + (unsigned long) section->sh_size); + goto exit_point; + } } isyms = (Elf_Internal_Sym *) cmalloc (number, sizeof (Elf_Internal_Sym)); @@ -4844,10 +4866,18 @@ get_64bit_elf_symbols (FILE * file, Elf_Internal_Sym * psym; unsigned int j; - /* Run some sanity checks first. */ - if (section->sh_entsize == 0) + if (section->sh_size == 0) { - error (_("sh_entsize is zero\n")); + if (num_syms_return != NULL) + * num_syms_return = 0; + return NULL; + } + + /* Run some sanity checks first. */ + if (section->sh_entsize == 0 || section->sh_entsize > section->sh_size) + { + error (_("Section %s has an invalid sh_entsize of 0x%lx\n"), + printable_section_name (section), (unsigned long) section->sh_entsize); goto exit_point; } @@ -4862,7 +4892,8 @@ get_64bit_elf_symbols (FILE * file, if (number * sizeof (Elf64_External_Sym) > section->sh_size + 1) { - error (_("Invalid sh_entsize\n")); + error (_("Size (0x%lx) of section %s is not a multiple of its sh_entsize (0x%lx)\n"), + section->sh_size, printable_section_name (section), section->sh_entsize); goto exit_point; } @@ -4881,6 +4912,14 @@ get_64bit_elf_symbols (FILE * file, _("symbol table section indicies")); if (shndx == NULL) goto exit_point; + else if (symtab_shndx_hdr->sh_size / sizeof (Elf_External_Sym_Shndx) < number) + { + error (_("Index section %s has an sh_size of 0x%lx - expected 0x%lx\n"), + printable_section_name (symtab_shndx_hdr), + (unsigned long) symtab_shndx_hdr->sh_size, + (unsigned long) section->sh_size); + goto exit_point; + } } isyms = (Elf_Internal_Sym *) cmalloc (number, sizeof (Elf_Internal_Sym)); @@ -5796,6 +5835,15 @@ process_section_groups (FILE * file) ? strtab + sym->st_name : _(""); } + /* PR 17531: file: loop. */ + if (section->sh_entsize > section->sh_size) + { + error (_("Section %s has sh_entsize (0x%lx) which is larger than its size (0x%lx)\n"), + printable_section_name (section), + section->sh_entsize, section->sh_size); + break; + } + start = (unsigned char *) get_data (NULL, file, section->sh_offset, 1, section->sh_size, _("section data"));