diff mbox

Fix dynamic-stack-buffer-overflow in linespec_lexer_lex_string

Message ID 20190524152102.GA19370@delia
State New, archived
Headers

Commit Message

Tom de Vries May 24, 2019, 3:21 p.m. UTC 
  Hi,

When compiling gdb with '-lasan -fsanitizer=address' and running tests with
'export ASAN_OPTIONS="detect_leaks=0:alloc_dealloc_mismatch=0"', I run into:
...
ERROR: GDB process no longer exists
UNRESOLVED: gdb.linespec/cpls-abi-tag.exp: \
  test_abi_tag: completion: at tag: tab complete "b test_abi_tag_function[abi:"
...

In more detail:
...
==3637==ERROR: AddressSanitizer: dynamic-stack-buffer-overflow on address \
  0x7fff5952bbdd at pc 0x000000fe5c57 bp 0x7fff5952af30 sp 0x7fff5952af28
READ of size 1 at 0x7fff5952bbdd thread T0
    #0 0xfe5c56 in linespec_lexer_lex_string src/gdb/linespec.c:727
    #1 0xfe7473 in linespec_lexer_lex_one src/gdb/linespec.c:946
    #2 0xfe799d in linespec_lexer_consume_token src/gdb/linespec.c:982
    #3 0xff446d in parse_linespec src/gdb/linespec.c:2564
    #4 0xff78be in linespec_complete(completion_tracker&, char const*, \
                   symbol_name_match_type) src/gdb/linespec.c:2961
    #5 0xb9299c in complete_address_and_linespec_locations \
                   src/gdb/completer.c:573
    #6 0xb93e90 in location_completer(cmd_list_element*, completion_tracker&, \
                   char const*, char const*) src/gdb/completer.c:919
    #7 0xb940c5 in location_completer_handle_brkchars src/gdb/completer.c:956
    #8 0xb957ec in complete_line_internal_normal_command \
                   src/gdb/completer.c:1208
    #9 0xb96507 in complete_line_internal_1 src/gdb/completer.c:1430
    #10 0xb965c2 in complete_line_internal src/gdb/completer.c:1449
    #11 0xb98630 in gdb_completion_word_break_characters_throw \
                    src/gdb/completer.c:1862
    #12 0xb98838 in gdb_completion_word_break_characters() \
                    src/gdb/completer.c:1897
    #13 0x16c6362 in _rl_find_completion_word src/readline/complete.c:943
    #14 0x16ca8d0 in rl_complete_internal src/readline/complete.c:1843
    #15 0x16c460c in rl_complete src/readline/complete.c:408
    #16 0x16b3368 in _rl_dispatch_subseq src/readline/readline.c:774
    #17 0x16b3092 in _rl_dispatch src/readline/readline.c:724
    #18 0x16b2939 in readline_internal_char src/readline/readline.c:552
    #19 0x16f1fb0 in rl_callback_read_char src/readline/callback.c:201
    #20 0xddc5a1 in gdb_rl_callback_read_char_wrapper_noexcept \
                    src/gdb/event-top.c:175
    #21 0xddc773 in gdb_rl_callback_read_char_wrapper src/gdb/event-top.c:192
    #22 0xddd9f5 in stdin_event_handler(int, void*) src/gdb/event-top.c:514
    #23 0xdd7d8f in handle_file_event src/gdb/event-loop.c:731
    #24 0xdd8607 in gdb_wait_for_event src/gdb/event-loop.c:857
    #25 0xdd629c in gdb_do_one_event() src/gdb/event-loop.c:321
    #26 0xdd6344 in start_event_loop() src/gdb/event-loop.c:370
    #27 0x10a7715 in captured_command_loop src/gdb/main.c:331
    #28 0x10aa548 in captured_main src/gdb/main.c:1173
    #29 0x10aa5d8 in gdb_main(captured_main_args*) src/gdb/main.c:1188
    #30 0x87bd35 in main src/gdb/gdb.c:32
    #31 0x7fb0364c6f89 in __libc_start_main (/lib64/libc.so.6+0x20f89)
    #32 0x87bb49 in _start (build/gdb/gdb+0x87bb49)

Address 0x7fff5952bbdd is located in stack of thread T0 at offset 557 in frame
    #0 0xb93702 in location_completer(cmd_list_element*, completion_tracker&, \
                   char const*, char const*) src/gdb/completer.c:831

  This frame has 4 object(s):
    [32, 40) 'copy'
    [96, 104) 'location'
    [160, 168) 'text'
    [224, 256) 'completion_info' <== Memory access at offset 557 overflows \
                                    this variable
HINT: this may be a false positive if your program uses some custom stack \
      unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: dynamic-stack-buffer-overflow \
         src/gdb/linespec.c:727 in linespec_lexer_lex_string
Shadow bytes around the buggy address:
  0x10006b29d720: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006b29d730: 00 00 00 00 00 00 f1 f1 f1 f1 00 f2 f2 f2 f2 f2
  0x10006b29d740: f2 f2 00 f2 f2 f2 f2 f2 f2 f2 00 f2 f2 f2 f2 f2
  0x10006b29d750: f2 f2 00 00 00 00 f3 f3 f3 f3 00 00 00 00 00 00
  0x10006b29d760: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10006b29d770: 00 00 00 00 ca ca ca ca 00 00 00[05]cb cb cb cb
  0x10006b29d780: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x10006b29d790: 00 f2 f2 f2 f2 f2 f2 f2 00 f2 f2 f2 f3 f3 f3 f3
  0x10006b29d7a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006b29d7b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10006b29d7c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
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
==3637==ABORTING
...

The problem happens in linespec_lexer_lex_string when lexing
"b test_abi_tag_function[abi:\0" (using a notation where we make the implicit
terminating \0 explicit).

We arrrive here with (PARSER_STREAM (parser)) == ":\0":
...
             /* Do not tokenize ABI tags such as "[abi:cxx11]".  */
             else if (PARSER_STREAM (parser) - start > 4
                      && startswith (PARSER_STREAM (parser) - 4, "[abi"))
               ++(PARSER_STREAM (parser));
...
and consume ':', after which we end up here and consume '\0':
...
         /* Advance the stream.  */
         ++(PARSER_STREAM (parser));
...
after which (PARSER_STREAM (parser)) points past the end of the string.

Fix this by removing the first "++(PARSER_STREAM (parser))", and add an assert
to the second one to detect moving past the end-of-string.

Build and tested on x86_64-linux.

OK for trunk?

Thanks,
- Tom

[gdb] Fix dynamic-stack-buffer-overflow in linespec_lexer_lex_string

gdb/ChangeLog:

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

	PR gdb/24611
	* linespec.c (linespec_lexer_lex_string): Remove incorrect
	"++(PARSER_STREAM (parser))" for "[abi"-prefixed colon.  Add assert.

---
 gdb/linespec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

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

Tom>  	      else if (PARSER_STREAM (parser) - start > 4
Tom>  		       && startswith (PARSER_STREAM (parser) - 4, "[abi"))
Tom> -		++(PARSER_STREAM (parser));
Tom> +		;

I think it's better for empty statements to be more verbose, like:

 else if (...)
   {
     /* Nothing.  */
   }

This is ok with this change.

Tom
diff mbox

Patch

diff --git a/gdb/linespec.c b/gdb/linespec.c
index f418e03b77..d28c390f13 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -760,7 +760,7 @@  linespec_lexer_lex_string (linespec_parser *parser)
 	      /* Do not tokenize ABI tags such as "[abi:cxx11]".  */
 	      else if (PARSER_STREAM (parser) - start > 4
 		       && startswith (PARSER_STREAM (parser) - 4, "[abi"))
-		++(PARSER_STREAM (parser));
+		;
 
 	      /* Do not tokenify if the input length so far is one
 		 (i.e, a single-letter drive name) and the next character
@@ -861,6 +861,7 @@  linespec_lexer_lex_string (linespec_parser *parser)
 	    }
 
 	  /* Advance the stream.  */
+	  gdb_assert (*(PARSER_STREAM (parser)) != '\0');
 	  ++(PARSER_STREAM (parser));
 	}
     }