From 19871f45ddfa7681f8f7585e73409f4fe5b51258 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Mon, 3 Jul 2017 22:00:32 +0930 Subject: [PATCH] strings: remove section/file size check This reverts most of 06803313754, 2005-07-05 Dmitry V. Levin change adding a check that section size doesn't exceed file size. As we've seen recently with mmo tests, decoded section size can easily exceed file size with formats that encode section data. I've also changed "strings" to use bfd_malloc_and_get_section, so that "strings" won't die on a malloc failure. I think it's better to continue on looking at other sections after failing to dump a section with fuzzed size. The testcases at https://bugzilla.altlinux.org/show_bug.cgi?id=5871 on a 32-bit host now produce $ strings -d --target=a.out-i386 /tmp/bfdkiller.dat strings: error: /tmp/bfdkiller.dat(.text) is too large (0xffffffff bytes) strings: /tmp/bfdkiller.dat: Reading section .text failed: Memory exhausted strings: /tmp/bfdkiller.dat: Reading section .data failed: File truncated org.ec $ strings -d --target=a.out-i386 /tmp/eclipse-state strings: /tmp/eclipse-state: Reading section .text failed: File truncated org.eclipse.osgi System Bundle [snip] * strings.c (filename_and_size_t): Delete. (strings_a_section): Don't check section size against file size. Use bdf_malloc_and_get_section. Report an error on failures. Replace arg param with filename and got_a_section param. (got_a_section): Move to.. (strings_object_file): ..an auto var here. Iterate over sections rather than calling bfd_map_over_sections. Adjust strings_a_section call. --- binutils/ChangeLog | 11 ++++++++ binutils/strings.c | 74 +++++++++++++----------------------------------------- 2 files changed, 28 insertions(+), 57 deletions(-) diff --git a/binutils/ChangeLog b/binutils/ChangeLog index a7be3db..0c1bf1c 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,14 @@ +2017-07-03 Alan Modra + + * strings.c (filename_and_size_t): Delete. + (strings_a_section): Don't check section size against file size. + Use bdf_malloc_and_get_section. Report an error on failures. + Replace arg param with filename and got_a_section param. + (got_a_section): Move to.. + (strings_object_file): ..an auto var here. Iterate over sections + rather than calling bfd_map_over_sections. Adjust strings_a_section + call. + 2017-07-02 Jan Kratochvil * dwarf.c: Include assert.h. diff --git a/binutils/strings.c b/binutils/strings.c index 77d89eb..3954473 100644 --- a/binutils/strings.c +++ b/binutils/strings.c @@ -108,9 +108,6 @@ static bfd_boolean print_filenames; /* TRUE means for object files scan only the data section. */ static bfd_boolean datasection_only; -/* TRUE if we found an initialized data section in the current file. */ -static bfd_boolean got_a_section; - /* The BFD object file format. */ static char *target; @@ -137,15 +134,6 @@ static struct option long_options[] = {NULL, 0, NULL, 0} }; -/* Records the size of a named file so that we - do not repeatedly run bfd_stat() on it. */ - -typedef struct -{ - const char * filename; - bfd_size_type filesize; -} filename_and_size_t; - static bfd_boolean strings_file (char *); static void print_strings (const char *, FILE *, file_ptr, int, int, char *); static void usage (FILE *, int) ATTRIBUTE_NORETURN; @@ -329,61 +317,33 @@ main (int argc, char **argv) return (exit_status); } -/* Scan section SECT of the file ABFD, whose printable name is in - ARG->filename and whose size might be in ARG->filesize. If it - contains initialized data set `got_a_section' and print the - strings in it. - - FIXME: We ought to be able to return error codes/messages for - certain conditions. */ +/* Scan section SECT of the file ABFD, whose printable name is + FILENAME. If it contains initialized data set GOT_A_SECTION and + print the strings in it. */ static void -strings_a_section (bfd *abfd, asection *sect, void *arg) +strings_a_section (bfd *abfd, asection *sect, const char *filename, + bfd_boolean *got_a_section) { - filename_and_size_t * filename_and_sizep; - bfd_size_type *filesizep; bfd_size_type sectsize; - void *mem; + bfd_byte *mem; if ((sect->flags & DATA_FLAGS) != DATA_FLAGS) return; sectsize = bfd_get_section_size (sect); - - if (sectsize <= 0) - return; - - /* Get the size of the file. This might have been cached for us. */ - filename_and_sizep = (filename_and_size_t *) arg; - filesizep = & filename_and_sizep->filesize; - - if (*filesizep == 0) - { - struct stat st; - - if (bfd_stat (abfd, &st)) - return; - - /* Cache the result so that we do not repeatedly stat this file. */ - *filesizep = st.st_size; - } - - /* Compare the size of the section against the size of the file. - If the section is bigger then the file must be corrupt and - we should not try dumping it. */ - if (sectsize >= *filesizep) + if (sectsize == 0) return; - mem = xmalloc (sectsize); - - if (bfd_get_section_contents (abfd, sect, mem, (file_ptr) 0, sectsize)) + if (!bfd_malloc_and_get_section (abfd, sect, &mem)) { - got_a_section = TRUE; - - print_strings (filename_and_sizep->filename, NULL, sect->filepos, - 0, sectsize, (char *) mem); + non_fatal (_("%s: Reading section %s failed: %s"), + filename, sect->name, bfd_errmsg (bfd_get_error ())); + return; } + *got_a_section = TRUE; + print_strings (filename, NULL, sect->filepos, 0, sectsize, (char *) mem); free (mem); } @@ -396,8 +356,9 @@ strings_a_section (bfd *abfd, asection *sect, void *arg) static bfd_boolean strings_object_file (const char *file) { - filename_and_size_t filename_and_size; bfd *abfd; + asection *s; + bfd_boolean got_a_section; abfd = bfd_openr (file, target); @@ -415,9 +376,8 @@ strings_object_file (const char *file) } got_a_section = FALSE; - filename_and_size.filename = file; - filename_and_size.filesize = 0; - bfd_map_over_sections (abfd, strings_a_section, & filename_and_size); + for (s = abfd->sections; s != NULL; s = s->next) + strings_a_section (abfd, s, file, &got_a_section); if (!bfd_close (abfd)) { -- 2.7.4