diff mbox

Fix heap-buffer-overflow in cp_find_first_component_aux

Message ID 20190525093527.GA2911@delia
State New, archived
Headers

Commit Message

Tom de Vries May 25, 2019, 9:35 a.m. UTC 
  Hi,

When compiling gdb with '-lasan -fsanitizer=address' and running tests with:
- export ASAN_OPTIONS="detect_leaks=0:alloc_dealloc_mismatch=0",
- target board cc-with-gdb-index,
- the "[gdb/testsuite] Fix gdb.base/break-probes.exp with native-gdbserver"
  commit reverted to avoid running into PR24617,
we get with gdb.arch/amd64-init-x87-values.exp:
...
==31229==ERROR: AddressSanitizer: heap-buffer-overflow on address \
  0x62500098c93c at pc 0x000000bcc748 bp 0x7ffe39487660 sp 0x7ffe39487658
READ of size 1 at 0x62500098c93c thread T0
    #0 0xbcc747 in cp_find_first_component_aux src/gdb/cp-support.c:999
    #1 0xbcc6e9 in cp_find_first_component(char const*) \
                   src/gdb/cp-support.c:977
    #2 0xcc2cf3 in mapped_index_base::build_name_components() \
                   src/gdb/dwarf2read.c:4499
    #3 0xcc3322 in dw2_expand_symtabs_matching_symbol src/gdb/dwarf2read.c:4552
    #4 0xcc817f in dw2_expand_symtabs_matching src/gdb/dwarf2read.c:5228
    #5 0xfe8f48 in iterate_over_all_matching_symtabs src/gdb/linespec.c:1147
    #6 0x1003506 in add_matching_symbols_to_info src/gdb/linespec.c:4413
    #7 0xffe21b in find_function_symbols src/gdb/linespec.c:3886
    #8 0xffe4a2 in find_linespec_symbols src/gdb/linespec.c:3914
    #9 0xfee3ad in linespec_parse_basic src/gdb/linespec.c:1865
    #10 0xff5128 in parse_linespec src/gdb/linespec.c:2655
    #11 0xff8872 in event_location_to_sals src/gdb/linespec.c:3150
    #12 0xff90a8 in decode_line_full(event_location const*, int, \
                    program_space*, symtab*, int, linespec_result*, \
		    char const*, char const*) src/gdb/linespec.c:3230
    #13 0x9ce449 in parse_breakpoint_sals src/gdb/breakpoint.c:9057
    #14 0x9ea022 in create_sals_from_location_default src/gdb/breakpoint.c:13708
    #15 0x9e2c1f in bkpt_create_sals_from_location src/gdb/breakpoint.c:12514
    #16 0x9cff06 in create_breakpoint(gdbarch*, event_location const*, \
                    char const*, int, char const*, int, int, bptype, int, \
		    auto_boolean, breakpoint_ops const*, int, int, int, \
		    unsigned int) src/gdb/breakpoint.c:9238
    #17 0x9d114a in break_command_1 src/gdb/breakpoint.c:9402
    #18 0x9d1b60 in break_command(char const*, int) src/gdb/breakpoint.c:9473
    #19 0xac96aa in do_const_cfunc src/gdb/cli/cli-decode.c:106
    #20 0xad0e5a in cmd_func(cmd_list_element*, char const*, int) \
                    src/gdb/cli/cli-decode.c:1892
    #21 0x15226f6 in execute_command(char const*, int) src/gdb/top.c:630
    #22 0xddde37 in command_handler(char const*) src/gdb/event-top.c:586
    #23 0xdde7c1 in command_line_handler(std::unique_ptr<char, \
                    gdb::xfree_deleter<char> >&&) src/gdb/event-top.c:773
    #24 0xddc9e8 in gdb_rl_callback_handler src/gdb/event-top.c:217
    #25 0x16f2198 in rl_callback_read_char src/readline/callback.c:220
    #26 0xddc5a1 in gdb_rl_callback_read_char_wrapper_noexcept \
                    src/gdb/event-top.c:175
    #27 0xddc773 in gdb_rl_callback_read_char_wrapper src/gdb/event-top.c:192
    #28 0xddd9f5 in stdin_event_handler(int, void*) src/gdb/event-top.c:514
    #29 0xdd7d8f in handle_file_event src/gdb/event-loop.c:731
    #30 0xdd8607 in gdb_wait_for_event src/gdb/event-loop.c:857
    #31 0xdd629c in gdb_do_one_event() src/gdb/event-loop.c:321
    #32 0xdd6344 in start_event_loop() src/gdb/event-loop.c:370
    #33 0x10a7715 in captured_command_loop src/gdb/main.c:331
    #34 0x10aa548 in captured_main src/gdb/main.c:1173
    #35 0x10aa5d8 in gdb_main(captured_main_args*) src/gdb/main.c:1188
    #36 0x87bd35 in main src/gdb/gdb.c:32
    #37 0x7f16e1434f89 in __libc_start_main (/lib64/libc.so.6+0x20f89)
    #38 0x87bb49 in _start (build/gdb/gdb+0x87bb49)

0x62500098c93c is located 0 bytes to the right of 8252-byte region \
  [0x62500098a900,0x62500098c93c)
allocated by thread T0 here:
    #0 0x7f16e359a600 in malloc (/usr/lib64/libasan.so.5+0xeb600)
    #1 0x1742ddf in bfd_malloc src/bfd/libbfd.c:275
    #2 0x1738824 in bfd_get_full_section_contents src/bfd/compress.c:253
    #3 0xe30044 in gdb_bfd_map_section(bfd_section*, unsigned long*) \
                   src/gdb/gdb_bfd.c:704
    #4 0xcb56bf in dwarf2_read_section(objfile*, dwarf2_section_info*) \
                   src/gdb/dwarf2read.c:2539
    #5 0xd5bcd0 in get_gdb_index_contents_from_section<dwarf2_per_objfile> \
                   src/gdb/dwarf2read.c:6217
    #6 0xd7fc7d in gdb::function_view<gdb::array_view<unsigned char const> \
                   (...) const src/gdb/common/function-view.h:284
    #7 0xd7fddd in gdb::function_view<gdb::array_view<unsigned char const> \
                   (...) src/gdb/common/function-view.h:278
    #8 0xd730cf in gdb::function_view<gdb::array_view<unsigned char const> \
                   (...) const src/gdb/common/function-view.h:247
    #9 0xcbc7ee in dwarf2_read_gdb_index src/gdb/dwarf2read.c:3582
    #10 0xcce731 in dwarf2_initialize_objfile(objfile*, dw_index_kind*) \
                    src/gdb/dwarf2read.c:6297
    #11 0xdb88c4 in elf_symfile_read src/gdb/elfread.c:1256
    #12 0x141262a in read_symbols src/gdb/symfile.c:798
    #13 0x14140a7 in syms_from_objfile_1 src/gdb/symfile.c:1000
    #14 0x1414393 in syms_from_objfile src/gdb/symfile.c:1017
    #15 0x1414fb7 in symbol_file_add_with_addrs src/gdb/symfile.c:1124
    #16 0x14159b7 in symbol_file_add_from_bfd(bfd*, char const*, \
                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
	             std::allocator<other_sections> >*, \
		     enum_flags<objfile_flag>, objfile*) src/gdb/symfile.c:1203
    #17 0x1415b6c in symbol_file_add(char const*,
                     enum_flags<symfile_add_flag>, std::vector<other_sections, \
		     std::allocator<other_sections> >*, \
		     enum_flags<objfile_flag>) src/gdb/symfile.c:1216
    #18 0x1415f2f in symbol_file_add_main_1 src/gdb/symfile.c:1240
    #19 0x1418599 in symbol_file_command(char const*, int) \
                     src/gdb/symfile.c:1675
    #20 0xde2fa6 in file_command src/gdb/exec.c:433
    #21 0xac96aa in do_const_cfunc src/gdb/cli/cli-decode.c:106
    #22 0xad0e5a in cmd_func(cmd_list_element*, char const*, int) \
                    src/gdb/cli/cli-decode.c:1892
    #23 0x15226f6 in execute_command(char const*, int) src/gdb/top.c:630
    #24 0xddde37 in command_handler(char const*) src/gdb/event-top.c:586
    #25 0xdde7c1 in command_line_handler(std::unique_ptr<char, \
                    gdb::xfree_deleter<char> >&&) src/gdb/event-top.c:773
    #26 0xddc9e8 in gdb_rl_callback_handler src/gdb/event-top.c:217
    #27 0x16f2198 in rl_callback_read_char src/readline/callback.c:220
    #28 0xddc5a1 in gdb_rl_callback_read_char_wrapper_noexcept \
                    src/gdb/event-top.c:175
    #29 0xddc773 in gdb_rl_callback_read_char_wrapper src/gdb/event-top.c:192

SUMMARY: AddressSanitizer: heap-buffer-overflow src/gdb/cp-support.c:999 in \
  cp_find_first_component_aux
Shadow bytes around the buggy address:
  0x0c4a801298d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a801298e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a801298f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80129900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c4a80129910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c4a80129920: 00 00 00 00 00 00 00[04]fa fa fa fa fa fa fa fa
  0x0c4a80129930: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129940: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129950: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c4a80129970: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==31229==ABORTING
...

The problem happens as follows.

The executable amd64-init-x87-values gets an index (due to target board
cc-with-gdb-index), which looks as follows:
...
Hex dump of section '.gdb_index':
  0x00000000 08000000 18000000 28000000 28000000 ........(...(...
  0x00000010 3c000000 3c200000 00000000 00000000 <...< ..........
  0x00000020 2e000000 00000000 d4004000 00000000 ..........@.....
  0x00000030 db004000 00000000 00000000 00000000 ..@.............
  0x00000040 00000000 00000000 00000000 00000000 ................
  0x00000050 00000000 00000000 00000000 00000000 ................
  ... more zeroes ...
  0x00002010 00000000 00000000 00000000 00000000 ................
  0x00002020 00000000 00000000 00000000 00000000 ................
  0x00002030 00000000 00000000 00000000          ............
...

The structure of this index is:
...
header       : [0x0, 0x18)     : size 0x18
culist       : [0x18 ,0x28)    : size 0x10
typesculist  : [0x28, 0x28)    : size 0x0
adress area  : [0x28, 0x3c)    : size 0x14
symbol table : [0x3c, 0x203c)  : size 0x2000
constant pool: [0x203c, 0x203c): size 0x0
EOF          : 0x203c
...

Note that the symbol table consists entirely of empty slots (where an empty
slot is a pair of 32-bit zeroes), and that the constant pool is empty.

The problem happens here in mapped_index_base::build_name_components:
...
  auto count = this->symbol_name_count ();
  for (offset_type idx = 0; idx < count; idx++)
    {
      if (this->symbol_name_slot_invalid (idx))
	continue;

      const char *name = this->symbol_name_at (idx);
...
when accessing the slot at idx == 0 in the symbol table,
symbol_name_slot_invalid returns false so we calculate name, which is
calculated using 'constant_pool + symbol_table[idx].name', which means we get
name == constant_pool.  And given that the constant pool is empty, name now
points past the memory allocated for the index, and when we access name[0] for
the first time cp_find_first_component_aux, we run into the
heap-buffer-overflow.

Fix this by fixing the definition of symbol_name_slot_invalid:
...
-    return bucket.name == 0 && bucket.vec;
+    return bucket.name == 0 && bucket.vec == 0;
...

Tested on x86_64-linux.

OK for trunk?

Thanks,
- Tom

[gdb] Fix heap-buffer-overflow in cp_find_first_component_aux

gdb/ChangeLog:

2019-05-25  Tom de Vries  <tdevries@suse.de>

	PR gdb/24618
	* dwarf2read.c (struct mapped_index::symbol_name_slot_invalid): Make
	sure an empty slot (defined by a 32-bit zero pair) is recognized as
	invalid.

---
 gdb/dwarf2read.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Tom Tromey May 29, 2019, 6:55 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Tom> [gdb] Fix heap-buffer-overflow in cp_find_first_component_aux

Tom> gdb/ChangeLog:

Tom> 2019-05-25  Tom de Vries  <tdevries@suse.de>

Tom> 	PR gdb/24618
Tom> 	* dwarf2read.c (struct mapped_index::symbol_name_slot_invalid): Make
Tom> 	sure an empty slot (defined by a 32-bit zero pair) is recognized as
Tom> 	invalid.

Thanks for the patch and the excellent analysis.

I looked at the history here a little and the earlier code did:

-      if (bucket.name == 0 && bucket.vec == 0)

... so this seems to have just been a simple error in the patch.

This is ok, please check it in.

Tom
Simon Marchi May 31, 2019, 12:53 p.m. UTC | #2 
On 2019-05-29 2:55 p.m., Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> [gdb] Fix heap-buffer-overflow in cp_find_first_component_aux
> 
> Tom> gdb/ChangeLog:
> 
> Tom> 2019-05-25  Tom de Vries  <tdevries@suse.de>
> 
> Tom> 	PR gdb/24618
> Tom> 	* dwarf2read.c (struct mapped_index::symbol_name_slot_invalid): Make
> Tom> 	sure an empty slot (defined by a 32-bit zero pair) is recognized as
> Tom> 	invalid.
> 
> Thanks for the patch and the excellent analysis.
> 
> I looked at the history here a little and the earlier code did:
> 
> -      if (bucket.name == 0 && bucket.vec == 0)
> 
> ... so this seems to have just been a simple error in the patch.
> 
> This is ok, please check it in.
> 
> Tom

Agreed, I was able to reproduce the failure, and the fix LGTM.

Simon
diff mbox

Patch

diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index f48b931a3f..aca56c07f0 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -212,7 +212,7 @@  struct mapped_index final : public mapped_index_base
   bool symbol_name_slot_invalid (offset_type idx) const override
   {
     const auto &bucket = this->symbol_table[idx];
-    return bucket.name == 0 && bucket.vec;
+    return bucket.name == 0 && bucket.vec == 0;
   }
 
   /* Convenience method to get at the name of the symbol at IDX in the