From f64e188b58f4aab4cbd03aa6e9fc1aa602546e26 Mon Sep 17 00:00:00 2001 From: Nick Clifton Date: Tue, 9 Dec 2014 12:42:18 +0000 Subject: [PATCH] More fixes for memory access violations triggered by fuzzed binaries. PR binutils/17512 * objdump.c (display_any_bfd): Avoid infinite loop closing and opening the same archive again and again. * archive64.c (bfd_elf64_archive_slurp_armap): Add range checks. * libbfd.c (safe_read_leb128): New function. * libbfd-in.h (safe_read_leb128): Add prototype. * libbfd.h: Regenerate. * elf-attrs.c (_bfd_elf_parse_attributes): Use safe_read_leb128. Check for an over-long subsection length. * elf.c (elf_parse_notes): Check that the namedata is long enough for the string comparison that is about to be performed. (elf_read_notes): Zero-terminate the note buffer. --- bfd/ChangeLog | 13 ++++++++++++ bfd/archive64.c | 11 +++++++++- bfd/elf-attrs.c | 15 +++++++------ bfd/elf.c | 53 +++++++++++++++++++++++++--------------------- bfd/libbfd-in.h | 2 ++ bfd/libbfd.c | 39 ++++++++++++++++++++++++++++++++++ bfd/libbfd.h | 2 ++ binutils/ChangeLog | 6 ++++++ binutils/objdump.c | 10 ++++++++- 9 files changed, 119 insertions(+), 32 deletions(-) diff --git a/bfd/ChangeLog b/bfd/ChangeLog index 974b8d8a6e7..874f46adff0 100644 --- a/bfd/ChangeLog +++ b/bfd/ChangeLog @@ -1,3 +1,16 @@ +2014-12-09 Nick Clifton + + PR binutils/17512 + * archive64.c (bfd_elf64_archive_slurp_armap): Add range checks. + * libbfd.c (safe_read_leb128): New function. + * libbfd-in.h (safe_read_leb128): Add prototype. + * libbfd.h: Regenerate. + * elf-attrs.c (_bfd_elf_parse_attributes): Use safe_read_leb128. + Check for an over-long subsection length. + * elf.c (elf_parse_notes): Check that the namedata is long enough + for the string comparison that is about to be performed. + (elf_read_notes): Zero-terminate the note buffer. + 2014-12-09 Alan Modra * elf64-ppc.c (sort_r_offset): Delete. diff --git a/bfd/archive64.c b/bfd/archive64.c index 6b87ec52028..9d29b90e646 100644 --- a/bfd/archive64.c +++ b/bfd/archive64.c @@ -46,6 +46,7 @@ bfd_elf64_archive_slurp_armap (bfd *abfd) struct areltdata *mapdata; bfd_byte int_buf[8]; char *stringbase; + char *stringend; bfd_byte *raw_armap = NULL; carsym *carsyms; bfd_size_type amt; @@ -92,11 +93,18 @@ bfd_elf64_archive_slurp_armap (bfd *abfd) ptrsize = 8 * nsymz; amt = carsym_size + stringsize + 1; + if (carsym_size < nsymz || ptrsize < nsymz || amt < nsymz) + { + bfd_set_error (bfd_error_malformed_archive); + return FALSE; + } ardata->symdefs = (struct carsym *) bfd_zalloc (abfd, amt); if (ardata->symdefs == NULL) return FALSE; carsyms = ardata->symdefs; stringbase = ((char *) ardata->symdefs) + carsym_size; + stringbase[stringsize] = 0; + stringend = stringbase + stringsize; raw_armap = (bfd_byte *) bfd_alloc (abfd, ptrsize); if (raw_armap == NULL) @@ -114,7 +122,8 @@ bfd_elf64_archive_slurp_armap (bfd *abfd) { carsyms->file_offset = bfd_getb64 (raw_armap + i * 8); carsyms->name = stringbase; - stringbase += strlen (stringbase) + 1; + if (stringbase < stringend) + stringbase += strlen (stringbase) + 1; ++carsyms; } *stringbase = '\0'; diff --git a/bfd/elf-attrs.c b/bfd/elf-attrs.c index 25f7e2672d7..8f76b6a8a41 100644 --- a/bfd/elf-attrs.c +++ b/bfd/elf-attrs.c @@ -492,7 +492,7 @@ _bfd_elf_parse_attributes (bfd *abfd, Elf_Internal_Shdr * hdr) bfd_vma subsection_len; bfd_byte *end; - tag = read_unsigned_leb128 (abfd, p, &n); + tag = safe_read_leb128 (abfd, p, &n, FALSE, p_end); p += n; if (p < p_end - 4) subsection_len = bfd_get_32 (abfd, p); @@ -506,6 +506,9 @@ _bfd_elf_parse_attributes (bfd *abfd, Elf_Internal_Shdr * hdr) section_len -= subsection_len; subsection_len -= n + 4; end = p + subsection_len; + /* PR 17512: file: 0e8c0c90. */ + if (end > p_end) + end = p_end; switch (tag) { case Tag_File: @@ -513,25 +516,25 @@ _bfd_elf_parse_attributes (bfd *abfd, Elf_Internal_Shdr * hdr) { int type; - tag = read_unsigned_leb128 (abfd, p, &n); + tag = safe_read_leb128 (abfd, p, &n, FALSE, end); p += n; type = _bfd_elf_obj_attrs_arg_type (abfd, vendor, tag); switch (type & (ATTR_TYPE_FLAG_INT_VAL | ATTR_TYPE_FLAG_STR_VAL)) { case ATTR_TYPE_FLAG_INT_VAL | ATTR_TYPE_FLAG_STR_VAL: - val = read_unsigned_leb128 (abfd, p, &n); + val = safe_read_leb128 (abfd, p, &n, FALSE, end); p += n; bfd_elf_add_obj_attr_int_string (abfd, vendor, tag, - val, (char *)p); + val, (char *) p); p += strlen ((char *)p) + 1; break; case ATTR_TYPE_FLAG_STR_VAL: bfd_elf_add_obj_attr_string (abfd, vendor, tag, - (char *)p); + (char *) p); p += strlen ((char *)p) + 1; break; case ATTR_TYPE_FLAG_INT_VAL: - val = read_unsigned_leb128 (abfd, p, &n); + val = safe_read_leb128 (abfd, p, &n, FALSE, end); p += n; bfd_elf_add_obj_attr_int (abfd, vendor, tag, val); break; diff --git a/bfd/elf.c b/bfd/elf.c index 405ec332b37..f6923b48a5b 100644 --- a/bfd/elf.c +++ b/bfd/elf.c @@ -9817,32 +9817,33 @@ elf_parse_notes (bfd *abfd, char *buf, size_t size, file_ptr offset) return TRUE; case bfd_core: - if (CONST_STRNEQ (in.namedata, "NetBSD-CORE")) - { - if (! elfcore_grok_netbsd_note (abfd, &in)) - return FALSE; - } - else if (CONST_STRNEQ (in.namedata, "OpenBSD")) - { - if (! elfcore_grok_openbsd_note (abfd, &in)) - return FALSE; - } - else if (CONST_STRNEQ (in.namedata, "QNX")) + { + struct { - if (! elfcore_grok_nto_note (abfd, &in)) - return FALSE; + const char * string; + bfd_boolean (* func)(bfd *, Elf_Internal_Note *); } - else if (CONST_STRNEQ (in.namedata, "SPU/")) + grokers[] = { - if (! elfcore_grok_spu_note (abfd, &in)) - return FALSE; - } - else - { - if (! elfcore_grok_note (abfd, &in)) - return FALSE; - } - break; + { "", elfcore_grok_note }, + { "NetBSD-CORE", elfcore_grok_netbsd_note }, + { "OpenBSD", elfcore_grok_openbsd_note }, + { "QNX", elfcore_grok_nto_note }, + { "SPU/", elfcore_grok_spu_note } + }; + int i; + + for (i = ARRAY_SIZE (grokers); i--;) + if (in.namesz >= sizeof grokers[i].string - 1 + && strncmp (in.namedata, grokers[i].string, + sizeof (grokers[i].string) - 1) == 0) + { + if (! grokers[i].func (abfd, & in)) + return FALSE; + break; + } + break; + } case bfd_object: if (in.namesz == sizeof "GNU" && strcmp (in.namedata, "GNU") == 0) @@ -9876,10 +9877,14 @@ elf_read_notes (bfd *abfd, file_ptr offset, bfd_size_type size) if (bfd_seek (abfd, offset, SEEK_SET) != 0) return FALSE; - buf = (char *) bfd_malloc (size); + buf = (char *) bfd_malloc (size + 1); if (buf == NULL) return FALSE; + /* PR 17512: file: ec08f814 + 0-termintate the buffer so that string searches will not overflow. */ + buf[size] = 0; + if (bfd_bread (buf, size, abfd) != size || !elf_parse_notes (abfd, buf, size, offset)) { diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h index f51bc1142dd..2800b72e3f7 100644 --- a/bfd/libbfd-in.h +++ b/bfd/libbfd-in.h @@ -839,3 +839,5 @@ extern void bfd_section_already_linked_table_traverse extern bfd_vma read_unsigned_leb128 (bfd *, bfd_byte *, unsigned int *); extern bfd_signed_vma read_signed_leb128 (bfd *, bfd_byte *, unsigned int *); +extern bfd_vma safe_read_leb128 (bfd *, bfd_byte *, unsigned int *, + bfd_boolean, const bfd_byte * const); diff --git a/bfd/libbfd.c b/bfd/libbfd.c index ade8e0fc833..c0792478e9d 100644 --- a/bfd/libbfd.c +++ b/bfd/libbfd.c @@ -1003,6 +1003,45 @@ read_unsigned_leb128 (bfd *abfd ATTRIBUTE_UNUSED, return result; } +/* Read in a LEB128 encoded value from ABFD starting at DATA. + If SIGN is true, return a signed LEB128 value. + If LENGTH_RETURN is not NULL, return in it the number of bytes read. + No bytes will be read at address END or beyond. */ + +bfd_vma +safe_read_leb128 (bfd *abfd ATTRIBUTE_UNUSED, + bfd_byte *data, + unsigned int *length_return, + bfd_boolean sign, + const bfd_byte * const end) +{ + bfd_vma result = 0; + unsigned int num_read = 0; + unsigned int shift = 0; + unsigned char byte = 0; + + while (data < end) + { + byte = bfd_get_8 (abfd, data); + data++; + num_read++; + + result |= ((bfd_vma) (byte & 0x7f)) << shift; + + shift += 7; + if ((byte & 0x80) == 0) + break; + } + + if (length_return != NULL) + *length_return = num_read; + + if (sign && (shift < 8 * sizeof (result)) && (byte & 0x40)) + result |= (bfd_vma) -1 << shift; + + return result; +} + /* Helper function for reading sleb128 encoded data. */ bfd_signed_vma diff --git a/bfd/libbfd.h b/bfd/libbfd.h index 14ee8c64f35..8687ece4e49 100644 --- a/bfd/libbfd.h +++ b/bfd/libbfd.h @@ -844,6 +844,8 @@ extern void bfd_section_already_linked_table_traverse extern bfd_vma read_unsigned_leb128 (bfd *, bfd_byte *, unsigned int *); extern bfd_signed_vma read_signed_leb128 (bfd *, bfd_byte *, unsigned int *); +extern bfd_vma safe_read_leb128 (bfd *, bfd_byte *, unsigned int *, + bfd_boolean, const bfd_byte * const); /* Extracted from init.c. */ /* Extracted from libbfd.c. */ bfd_boolean bfd_write_bigendian_4byte_int (bfd *, unsigned int); diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 7404a15df0f..845eed4754e 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,9 @@ +2014-12-09 Nick Clifton + + PR binutils/17512 + * objdump.c (display_any_bfd): Avoid infinite loop closing and + opening the same archive again and again. + 2014-12-09 Chen Gang * windres.c (open_file_search): Free path buffer on failure. diff --git a/binutils/objdump.c b/binutils/objdump.c index b43d11171d5..ee3a0840f0e 100644 --- a/binutils/objdump.c +++ b/binutils/objdump.c @@ -3426,7 +3426,15 @@ display_any_bfd (bfd *file, int level) display_any_bfd (arfile, level + 1); if (last_arfile != NULL) - bfd_close (last_arfile); + { + bfd_close (last_arfile); + /* PR 17512: file: ac585d01. */ + if (arfile == last_arfile) + { + last_arfile = NULL; + break; + } + } last_arfile = arfile; } -- 2.30.2