[Bug gdb/23657] New: Out of bound memory access

[jxx13 at psu dot edu]

https://sourceware.org/bugzilla/show_bug.cgi?id=23657

Bug ID: 23657
Summary: Out of bound memory access
Product: gdb
Version: HEAD
Status: UNCONFIRMED
Severity: critical
Priority: P2
Component: gdb
Assignee: unassigned at sourceware dot org
Reporter: jxx13 at psu dot edu
Target Milestone: ---

Created attachment 11246
--> https://sourceware.org/bugzilla/attachment.cgi?id=11246&action=edit
Sample to trigger the bug

Our fuzzer finds the following bug:

dwarf2read.c:19513:

19508 if (str_offset >= sect->size)
19509 error (_("%s pointing outside of %s section [in module %s]"),
19510 form_name, sect_name, bfd_get_filename (abfd));
19511 gdb_assert (HOST_CHAR_BIT == 8);
19512 if (sect->buffer[str_offset] == '\0')

Line 19508 compares the access index with the size of section. This size, which
is specified in the target, can be arbitrary value. Making it to be
0xffffffffffffffff (on X64), the comparison never works. This leads to out of
bound access at line 19512.

An test case running on x86_64 CentOS Linux 7 is attached. It can stably crash
the most recent version of GDB on github.

Stack trace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000586cb3 in read_indirect_string_at_offset_from (objfile=<optimized
out>, str_offset=<optimized out>, sect=0xfe5db8, form_name=0x900f18
"DW_FORM_strp", sect_name=0x8fb438 ".debug_str", abfd=<optimized out>) at
dwarf2read.c:19513
19513 return NULL;
Missing separate debuginfos, use: debuginfo-install expat-2.1.0-10.el7_3.x86_64
glibc-2.17-196.el7.x86_64 gmp-6.0.0-12.el7_1.x86_64 libgcc-4.8.5-16.el7.x86_64
libstdc++-4.8.5-16.el7.x86_64 mpfr-3.1.1-4.el7.x86_64
ncurses-libs-5.9-13.20130511.el7.x86_64 python-libs-2.7.5-58.el7.x86_64
xz-libs-5.2.2-1.el7.x86_64
(gdb) info stack
#0 0x0000000000586cb3 in read_indirect_string_at_offset_from
(objfile=<optimized out>, str_offset=<optimized out>, sect=0xfe5db8,
form_name=0x900f18 "DW_FORM_strp", sect_name=0x8fb438 ".debug_str",
abfd=<optimized out>) at dwarf2read.c:19513
#1 0x0000000000589919 in read_indirect_line_string_at_offset
(str_offset=<optimized out>, abfd=<optimized out>,
dwarf2_per_objfile=<optimized out>) at dwarf2read.c:19539
#2 read_indirect_line_string (bytes_read_ptr=<optimized out>,
cu_header=<optimized out>, buf=<optimized out>, abfd=<optimized out>,
dwarf2_per_objfile=<optimized out>) at dwarf2read.c:19595
#3 read_attribute_value (reader=***@entry=0x7fffffffdd20,
attr=***@entry=0x7fffffffda60, form=14, implicit_const=-1,
info_ptr=0x7fffef2d6773 "S\227\003") at dwarf2read.c:19077
#4 0x000000000058d179 in read_attribute (info_ptr=<optimized out>,
abbrev=<optimized out>, attr=0x7fffffffda60, reader=0x7fffffffdd20) at
dwarf2read.c:19242
#5 partial_die_info::read (this=***@entry=0x7fffffffdb30,
reader=***@entry=0x7fffffffdd20, abbrev=..., info_ptr=<optimized out>) at
dwarf2read.c:18562
#6 0x000000000058d972 in load_partial_dies
(reader=***@entry=0x7fffffffdd20, info_ptr=***@entry=0x7fffef2d6772
"\002S\227\003", building_psymtab=***@entry=1) at
dwarf2read.c:18370
#7 0x000000000059e11b in process_psymtab_comp_unit_reader
(reader=***@entry=0x7fffffffdd20, info_ptr=0x7fffef2d6772 "\002S\227\003",
comp_unit_die=0x1014d40, has_children=<optimized out>,
data=***@entry=0x7fffffffddb0) at dwarf2read.c:8030
#8 0x00000000005903f6 in init_cutu_and_read_dies
(this_cu=***@entry=0xfe6070, abbrev_table=<optimized out>,
***@entry=0x0, use_existing_cu=***@entry=0,
keep=***@entry=0, skip_partial=***@entry=false,
die_reader_func=0x59dd60 <process_psymtab_comp_unit_reader(die_reader_specs
const*, gdb_byte const*, die_info*, int, void*)>, data=0x7fffffffddb0) at
dwarf2read.c:7664
#9 0x0000000000593c3b in process_psymtab_comp_unit (this_cu=0xfe6070,
want_partial_unit=***@entry=0,
pretend_language=***@entry=language_minimal) at dwarf2read.c:8121
#10 0x00000000005a2988 in dwarf2_build_psymtabs_hard
(dwarf2_per_objfile=0xfe5ca0) at dwarf2read.c:8481
#11 dwarf2_build_psymtabs (objfile=0xfda160) at dwarf2read.c:6305
#12 0x00000000006469ec in require_partial_symbols
(objfile=***@entry=0xfda160, verbose=***@entry=0) at psymtab.c:86
#13 0x0000000000697b45 in read_symbols (objfile=***@entry=0xfda160,
add_flags=..., ***@entry=...) at symfile.c:817
#14 0x0000000000697373 in syms_from_objfile_1 (add_flags=...,
addrs=0x7fffffffdf70, objfile=0xfda160) at symfile.c:996
#15 syms_from_objfile (add_flags=..., addrs=0x20, objfile=0xfda160) at
symfile.c:1012
#16 symbol_file_add_with_addrs (abfd=<optimized out>,
name=***@entry=0x7fffffffe567 "/tmp/binutils-2.30/binutils/objdump",
add_flags=..., ***@entry=..., addrs=***@entry=0x0, flags=...,
***@entry=..., parent=***@entry=0x0) at symfile.c:1119
#17 0x00000000006986ca in symbol_file_add_from_bfd (parent=0x0, flags=...,
addrs=0x0, add_flags=..., name=0x7fffffffe567
"/tmp/binutils-2.30/binutils/objdump", abfd=<optimized out>) at symfile.c:1204
#18 symbol_file_add (name=***@entry=0x7fffffffe567
"/tmp/binutils-2.30/binutils/objdump", add_flags=***@entry=...,
addrs=***@entry=0x0, flags=***@entry=...) at symfile.c:1217
#19 0x0000000000698742 in symbol_file_add_main_1
(args=***@entry=0x7fffffffe567 "/tmp/binutils-2.30/binutils/objdump",
add_flags=..., flags=***@entry=..., reloff=***@entry=0) at symfile.c:1240
#20 0x000000000069878d in symbol_file_add_main (args=***@entry=0x7fffffffe567
"/tmp/binutils-2.30/binutils/objdump", add_flags=..., ***@entry=...) at
symfile.c:1231
#21 0x0000000000617663 in symbol_file_add_main_adapter
(arg=***@entry=0x7fffffffe567 "/tmp/binutils-2.30/binutils/objdump",
from_tty=***@entry=1) at main.c:403
#22 0x00000000006176f8 in catch_command_errors (command=***@entry=0x617650
<symbol_file_add_main_adapter(char const*, int)>, arg=***@entry=0x7fffffffe567
"/tmp/binutils-2.30/binutils/objdump", from_tty=1) at main.c:379
#23 0x000000000061859d in captured_main_1 (context=0x7fffffffe0b0,
this=<optimized out>) at main.c:1053
#24 captured_main (data=0x7fffffffe0b0) at main.c:1163
#25 gdb_main (args=***@entry=0x7fffffffe1e0) at main.c:1189
#26 0x000000000040f1d5 in main (argc=<optimized out>, argv=<optimized out>) at
gdb.c:32

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23657

Tom Tromey <tromey at sourceware dot org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2018-09-15
CC| |nickc at redhat dot com,
| |tromey at sourceware dot org
Ever confirmed|0 |1

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
IIUC the problem is that sect->size comes from a header somewhere,
but does not reflect the true size of the section. It took me a while
to understand this, because from the report I thought you meant that the
comparison could not possibly be true -- but really it could be ==.

I am not sure where the correct fix is.
I notice readelf gives a warning for this file:

readelf: /tmp/pr-23657: Warning: Size of section 31 is larger than the entire
file!

So maybe gdb could do this check.
Or maybe BFD ought to issue an error because the file is corrupted.

CCing Nick - Nick, do you think this should be handled in BFD?

I couldn't reproduce directly but valgrind shows the error.

[nickc at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23657

--- Comment #2 from Nick Clifton <nickc at redhat dot com> ---
Hi Tom,

Post by tromey at sourceware dot org
I am not sure where the correct fix is.

So does objdump, if you use an option which tries to reference
into the debug string section:

% objdump -S objdump
objdump: error: objdump(.debug_str) is too large (0xffffffffffffffff bytes)

Post by tromey at sourceware dot org
So maybe gdb could do this check.
Or maybe BFD ought to issue an error because the file is corrupted.
CCing Nick - Nick, do you think this should be handled in BFD?

Both would be my suggestion. You can never be too paranoid...

There is also the caveat that the test "if (str_offset >= sect->size)"
might be wrong if the .debug_str section is compressed. (I am unfamiliar
with the gdb sources, so I do not know if sect->size is the size from the
ELF header, or the size after decompression).

Cheers
Nick

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23657

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Nick Clifton from comment #2)

Post by nickc at redhat dot com
There is also the caveat that the test "if (str_offset >= sect->size)"
might be wrong if the .debug_str section is compressed. (I am unfamiliar
with the gdb sources, so I do not know if sect->size is the size from the
ELF header, or the size after decompression).

For a compressed section it comes from:

descriptor->size = bfd_get_section_size (sectp);

Where would a section size sanity check belong in BFD?
Maybe we'd only want it for sections we actually read, so
bfd_get_full_section_contents (and then a copy in gdb since
gdb can mmap sections).

I wonder if there are other sanity checks that should be done.

[nickc at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=23657

--- Comment #4 from Nick Clifton <nickc at redhat dot com> ---
Hi Tom,

(In reply to Tom Tromey from comment #3)

Post by tromey at sourceware dot org
Where would a section size sanity check belong in BFD?

Mmmm, well I think that when the file is first read in would
be the most reasonable place. I am going to experiment with
a patch to update elf_swap_shdr_in (in bfd/elfcode.h) and see
how that fairs.

Post by tromey at sourceware dot org
Maybe we'd only want it for sections we actually read, so
bfd_get_full_section_contents (and then a copy in gdb since
gdb can mmap sections).

That would make sense, although I think that we whould also
issue a warning message (but not an error) if we detect
something funny going on when the section headers are read in.

Cheers
Nick

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=23657

--- Comment #5 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Nick Clifton <***@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8ff71a9c80cfcf64c54d4ae938c644b1b1ea19fb

commit 8ff71a9c80cfcf64c54d4ae938c644b1b1ea19fb
Author: Nick Clifton <***@redhat.com>
Date: Tue Sep 18 16:54:07 2018 +0100

Add a warning to the bfd library for when it encounters an ELF file with an
invalid section size.

PR 23657
* elfcode.h (elf_swap_shdr_in): Generate a warning message if an
ELF section has contents and size larger than the file size.