From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 933 invoked by alias); 10 Jun 2019 18:36:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 720 invoked by uid 89); 10 Jun 2019 18:36:37 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: mx1.suse.de Received: from mx2.suse.de (HELO mx1.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 10 Jun 2019 18:36:35 +0000 Received: from relay2.suse.de (unknown [195.135.220.254]) by mx1.suse.de (Postfix) with ESMTP id 5FC35AF99; Mon, 10 Jun 2019 18:36:33 +0000 (UTC) Subject: [PING][PATCH][readline] Fix heap-buffer-overflow in update_line From: Tom de Vries To: gdb-patches@sourceware.org Cc: chet.ramey@case.edu References: <20190525112458.GA3838@delia> Message-ID: Date: Mon, 10 Jun 2019 18:36:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190525112458.GA3838@delia> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2019-06/txt/msg00197.txt.bz2 On 25-05-19 13:24, Tom de Vries wrote: > Hi, > > When: > - building trunk gdb with '-fsanitize=address -lasan', > - running gdb tests with "export ASAN_OPTIONS=detect_leaks=0", > I run into a heap-buffer-overflow failure for > gdb.base/utf8-identifiers.exp. > > In more detail, the libasan error report looks like this: > ... > ================================================================= > ==22340==ERROR: AddressSanitizer: heap-buffer-overflow on address > 0x619000054a80 at pc 0x7fcd0306b4c9 bp 0x7fffb1a8d880 sp 0x7fffb1a8d030 > READ of size 32766 at 0x619000054a80 thread T0 > #0 0x7fcd0306b4c8 (/usr/lib64/libasan.so.4+0xae4c8) > #1 0x15f12a1 in update_line > /data/gdb_versions/devel/src/readline/display.c:1377 > #2 0x15f03cb in rl_redisplay > /data/gdb_versions/devel/src/readline/display.c:1204 > #3 0x15bf932 in readline_internal_setup > /data/gdb_versions/devel/src/readline/readline.c:394 > #4 0x15fe723 in _rl_callback_newline > /data/gdb_versions/devel/src/readline/callback.c:89 > #5 0x15fe7ef in rl_callback_handler_install > /data/gdb_versions/devel/src/readline/callback.c:102 > #6 0xd7bce6 in gdb_rl_callback_handler_install(char const*) > /data/gdb_versions/devel/src/gdb/event-top.c:319 > #7 0xd7c0c6 in display_gdb_prompt(char const*) > /data/gdb_versions/devel/src/gdb/event-top.c:409 > #8 0xd7d6c1 in command_line_handler(std::unique_ptr gdb::xfree_deleter >&&) > /data/gdb_versions/devel/src/gdb/event-top.c:776 > #9 0xd7b92a in gdb_rl_callback_handler > /data/gdb_versions/devel/src/gdb/event-top.c:217 > #10 0x15ff479 in rl_callback_read_char > /data/gdb_versions/devel/src/readline/callback.c:220 > #11 0xd7b4d5 in gdb_rl_callback_read_char_wrapper_noexcept > /data/gdb_versions/devel/src/gdb/event-top.c:175 > #12 0xd7b6b5 in gdb_rl_callback_read_char_wrapper > /data/gdb_versions/devel/src/gdb/event-top.c:192 > #13 0xd7c8aa in stdin_event_handler(int, void*) > /data/gdb_versions/devel/src/gdb/event-top.c:514 > #14 0xd76ca7 in handle_file_event > /data/gdb_versions/devel/src/gdb/event-loop.c:731 > #15 0xd7751f in gdb_wait_for_event > /data/gdb_versions/devel/src/gdb/event-loop.c:857 > #16 0xd7547e in gdb_do_one_event() > /data/gdb_versions/devel/src/gdb/event-loop.c:321 > #17 0xd75526 in start_event_loop() > /data/gdb_versions/devel/src/gdb/event-loop.c:370 > #18 0x101b04c in captured_command_loop > /data/gdb_versions/devel/src/gdb/main.c:331 > #19 0x101de73 in captured_main > /data/gdb_versions/devel/src/gdb/main.c:1173 > #20 0x101df03 in gdb_main(captured_main_args*) > /data/gdb_versions/devel/src/gdb/main.c:1188 > #21 0x872dba in main /data/gdb_versions/devel/src/gdb/gdb.c:32 > #22 0x7fcd00f2ff49 in __libc_start_main (/lib64/libc.so.6+0x20f49) > #23 0x872bc9 in _start (/data/gdb_versions/devel/build/gdb/gdb+0x872bc9) > > 0x619000054a80 is located 0 bytes to the right of 1024-byte region > [0x619000054680,0x619000054a80) > allocated by thread T0 here: > #0 0x7fcd03099510 in malloc (/usr/lib64/libasan.so.4+0xdc510) > #1 0xae0078 in xmalloc > /data/gdb_versions/devel/src/gdb/common/common-utils.c:44 > #2 0x15eaccb in init_line_structures > /data/gdb_versions/devel/src/readline/display.c:458 > #3 0x15eb4d8 in rl_redisplay > /data/gdb_versions/devel/src/readline/display.c:526 > #4 0x15bf932 in readline_internal_setup > /data/gdb_versions/devel/src/readline/readline.c:394 > #5 0x15fe723 in _rl_callback_newline > /data/gdb_versions/devel/src/readline/callback.c:89 > #6 0x15fe7ef in rl_callback_handler_install > /data/gdb_versions/devel/src/readline/callback.c:102 > #7 0xd7bce6 in gdb_rl_callback_handler_install(char const*) > /data/gdb_versions/devel/src/gdb/event-top.c:319 > #8 0xd7c0c6 in display_gdb_prompt(char const*) > /data/gdb_versions/devel/src/gdb/event-top.c:409 > #9 0xaa041b in cli_interp_base::pre_command_loop() > /data/gdb_versions/devel/src/gdb/cli/cli-interp.c:286 > #10 0xf5342a in interp_pre_command_loop(interp*) > /data/gdb_versions/devel/src/gdb/interps.c:320 > #11 0x101b047 in captured_command_loop > /data/gdb_versions/devel/src/gdb/main.c:328 > #12 0x101de73 in captured_main > /data/gdb_versions/devel/src/gdb/main.c:1173 > #13 0x101df03 in gdb_main(captured_main_args*) > /data/gdb_versions/devel/src/gdb/main.c:1188 > #14 0x872dba in main /data/gdb_versions/devel/src/gdb/gdb.c:32 > #15 0x7fcd00f2ff49 in __libc_start_main (/lib64/libc.so.6+0x20f49) > > SUMMARY: AddressSanitizer: heap-buffer-overflow > (/usr/lib64/libasan.so.4+0xae4c8) > Shadow bytes around the buggy address: > 0x0c3280002900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c3280002910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c3280002920: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c3280002930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c3280002940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > =>0x0c3280002950:[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > 0x0c3280002960: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa > 0x0c3280002970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c3280002980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c3280002990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x0c32800029a0: 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 > ==22340==ABORTING > ... > > I've written an assert in rl_redisplay that formulates the error condition: > ... > @@ -1387,6 +1389,10 @@ rl_redisplay (void) > cpos_adjusted = 0; > + assert (last_lmargin + (_rl_screenwidth + visible_wrap_offset) > + <= line_size); > + assert (lmargin + (_rl_screenwidth + (lmargin ? 0 : wrap_offset)) > + <= line_size); > update_line (&visible_line[last_lmargin], > &invisible_line[lmargin], > 0, > _rl_screenwidth + visible_wrap_offset, > _rl_screenwidth + (lmargin ? 0 : wrap_offset), > 0); > ... > which triggers without needing the address sanitizer (or even an executable), > like this: > ... > $ TERM=dumb gdb -q -ex "set width 0" > gdb: src/display.c:1393: rl_redisplay: Assertion > `last_lmargin + (_rl_screenwidth + visible_wrap_offset) <= line_size' > failed. > Aborted (core dumped) > ... > > The basic problem is this: visible_line and invisible_line have length > line_size, but the update_line call assumes that line_size is at least > _rl_screenwidth + 1. Executing "set width 0" sets _rl_screenwidth to 32766 but > doesn't affect line_size, which is initialized to 1024. > > Fix this by ensuring in init_line_structures and rl_redisplay that line_size > is at least _rl_screenwidth + 1. > > Tested on x86_64-linux. > > Reviewed by readline maintainer ( > https://sourceware.org/ml/gdb-patches/2019-05/msg00566.html ). > > OK for binutils-gdb repo copy of readline? > Ping. Thanks, - Tom > [readline] Fix heap-buffer-overflow in update_line > > readline/ChangeLog.gdb: > > 2019-05-25 Tom de Vries > Chet Ramey > > PR cli/24514 > * readline/display.c (init_line_structures, rl_redisplay): Ensure > line_size is at least _rl_screenwidth + 1. > > --- > readline/display.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/readline/display.c b/readline/display.c > index 9044305797..842adf5067 100644 > --- a/readline/display.c > +++ b/readline/display.c > @@ -450,6 +450,9 @@ init_line_structures (minsize) > { > register int n; > > + if (minsize <= _rl_screenwidth) /* XXX - for gdb */ > + minsize = _rl_screenwidth + 1; > + > if (invisible_line == 0) /* initialize it */ > { > if (line_size < minsize) > @@ -526,6 +529,8 @@ rl_redisplay () > init_line_structures (0); > rl_on_new_line (); > } > + else if (line_size <= _rl_screenwidth) > + init_line_structures (_rl_screenwidth + 1); > > /* Draw the line into the buffer. */ > cpos_buffer_position = -1; >