[readline] Fix heap-buffer-overflow in update_line

Message ID	20190525112458.GA3838@delia
State	New, archived
Headers	Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk Sender: gdb-patches-owner@sourceware.org Date: Sat, 25 May 2019 13:24:59 +0200 From: Tom de Vries <tdevries@suse.de> To: gdb-patches@sourceware.org Cc: chet.ramey@case.edu Subject: [PATCH][readline] Fix heap-buffer-overflow in update_line Message-ID: <20190525112458.GA3838@delia> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline User-Agent: Mutt/1.10.1 (2018-07-13)

Message ID

20190525112458.GA3838@delia

State

New, archived

Headers

Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
Sender: gdb-patches-owner@sourceware.org
Date: Sat, 25 May 2019 13:24:59 +0200
From: Tom de Vries <tdevries@suse.de>
To: gdb-patches@sourceware.org
Cc: chet.ramey@case.edu
Subject: [PATCH][readline] Fix heap-buffer-overflow in update_line
Message-ID: <20190525112458.GA3838@delia>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.10.1 (2018-07-13)

Commit Message

Tom de Vries May 25, 2019, 11:24 a.m. UTC

  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:
...

Comments

Tom de Vries June 10, 2019, 6:36 p.m. UTC | #1

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<char,
> gdb::xfree_deleter<char> >&&)
> /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  <tdevries@suse.de>
> 	    Chet Ramey  <chet.ramey@case.edu>
> 
> 	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;
>

Tom de Vries June 17, 2019, 9:38 a.m. UTC | #2

On 10-06-19 20:36, Tom de Vries wrote:
> 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<char,
>> gdb::xfree_deleter<char> >&&)
>> /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^2.

Thanks,
- Tom

>> [readline] Fix heap-buffer-overflow in update_line
>>
>> readline/ChangeLog.gdb:
>>
>> 2019-05-25  Tom de Vries  <tdevries@suse.de>
>> 	    Chet Ramey  <chet.ramey@case.edu>
>>
>> 	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;
>>

Tom de Vries July 4, 2019, 6:09 a.m. UTC | #3

On 17-06-19 11:38, Tom de Vries wrote:
> On 10-06-19 20:36, Tom de Vries wrote:
>> 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<char,
>>> gdb::xfree_deleter<char> >&&)
>>> /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^3.

Thanks,
- Tom

>>> [readline] Fix heap-buffer-overflow in update_line
>>>
>>> readline/ChangeLog.gdb:
>>>
>>> 2019-05-25  Tom de Vries  <tdevries@suse.de>
>>> 	    Chet Ramey  <chet.ramey@case.edu>
>>>
>>> 	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;
>>>

Tom Tromey July 11, 2019, 8:41 p.m. UTC | #4

>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:

Apologies for the delay on this.

Tom> Ping^3.

>>>> 2019-05-25  Tom de Vries  <tdevries@suse.de>
>>>> Chet Ramey  <chet.ramey@case.edu>
>>>> 
>>>> PR cli/24514
>>>> * readline/display.c (init_line_structures, rl_redisplay): Ensure
>>>> line_size is at	least _rl_screenwidth + 1.

I don't know this part of readline very well.  However, if Chet approved
it and/or put it into upstream readline, then it is fine for gdb.

thanks,
Tom

Chet Ramey July 12, 2019, 1:35 p.m. UTC | #5

On 7/11/19 4:41 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Apologies for the delay on this.
> 
> Tom> Ping^3.
> 
>>>>> 2019-05-25  Tom de Vries  <tdevries@suse.de>
>>>>> Chet Ramey  <chet.ramey@case.edu>
>>>>>
>>>>> PR cli/24514
>>>>> * readline/display.c (init_line_structures, rl_redisplay): Ensure
>>>>> line_size is at	least _rl_screenwidth + 1.
> 
> I don't know this part of readline very well.  However, if Chet approved
> it and/or put it into upstream readline, then it is fine for gdb.

I did.

diff mbox

Patch

=================================================================
==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<char,
gdb::xfree_deleter<char> >&&)
/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?

Thanks,
- Tom

OK for trunk?

Thanks,
- Tom

[readline] Fix heap-buffer-overflow in update_line

readline/ChangeLog.gdb:

2019-05-25  Tom de Vries  <tdevries@suse.de>
	    Chet Ramey  <chet.ramey@case.edu>

	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;