From 4b04bba2eb6b646e11a2c38c77667875b3db6828 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Sun, 1 Oct 2017 12:07:59 +1030 Subject: [PATCH] PR22047, Heap out of bounds read in parse_comp_unit Like the PR22230 fix, we can allocate a buffer with an extra byte rather than letting bfd_simple_get_relocated_section_contents malloc and return a buffer. Much better than allocating another buffer afterwards. PR 22047 * dwarf2.c (read_section): Allocate buffer with extra byte for bfd_simple_get_relocated_section_contents rather than copying afterwards. --- bfd/ChangeLog | 7 +++++++ bfd/dwarf2.c | 53 +++++++++++++++-------------------------------------- 2 files changed, 22 insertions(+), 38 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 8b44ceb..9a6af67 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,10 @@ +2017-10-01 Alan Modra + + PR 22047 + * dwarf2.c (read_section): Allocate buffer with extra byte for + bfd_simple_get_relocated_section_contents rather than copying + afterwards. + 2017-09-29 Alan Modra * merge.c (merge_strings): Return FALSE on malloc failure. diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c index be415e3..4cf50cc 100644 --- a/bfd/dwarf2.c +++ b/bfd/dwarf2.c @@ -526,9 +526,10 @@ read_section (bfd * abfd, { asection *msec; const char *section_name = sec->uncompressed_name; + bfd_byte *contents = *section_buffer; /* The section may have already been read. */ - if (*section_buffer == NULL) + if (contents == NULL) { msec = bfd_get_section_by_name (abfd, section_name); if (! msec) @@ -546,45 +547,21 @@ read_section (bfd * abfd, } *section_size = msec->rawsize ? msec->rawsize : msec->size; - if (syms) - { - *section_buffer - = bfd_simple_get_relocated_section_contents (abfd, msec, NULL, syms); - if (! *section_buffer) - return FALSE; - } - else - { - *section_buffer = (bfd_byte *) bfd_malloc (*section_size); - if (! *section_buffer) - return FALSE; - if (! bfd_get_section_contents (abfd, msec, *section_buffer, - 0, *section_size)) - return FALSE; - } - - /* Paranoia - if we are reading in a string section, make sure that it - is NUL terminated. This is to prevent string functions from running - off the end of the buffer. Note - knowing the size of the buffer is - not enough as some functions, eg strchr, do not have a range limited - equivalent. - - FIXME: We ought to use a flag in the dwarf_debug_sections[] table to - determine the nature of a debug section, rather than checking the - section name as we do here. */ - if (*section_size > 0 - && (*section_buffer)[*section_size - 1] != 0 - && (strstr (section_name, "_str") || strstr (section_name, "names"))) + /* Paranoia - alloc one extra so that we can make sure a string + section is NUL terminated. */ + contents = (bfd_byte *) bfd_malloc (*section_size + 1); + if (contents == NULL) + return FALSE; + if (syms + ? !bfd_simple_get_relocated_section_contents (abfd, msec, contents, + syms) + : !bfd_get_section_contents (abfd, msec, contents, 0, *section_size)) { - bfd_byte * new_buffer = malloc (*section_size + 1); - - _bfd_error_handler (_("warning: dwarf string section '%s' is not NUL terminated"), - section_name); - memcpy (new_buffer, *section_buffer, *section_size); - new_buffer[*section_size] = 0; - free (*section_buffer); - *section_buffer = new_buffer; + free (contents); + return FALSE; } + contents[*section_size] = 0; + *section_buffer = contents; } /* It is possible to get a bad value for the offset into the section -- 2.7.4