Re: [Bug-readline] heap-buffer-overflow in update_line

[Tom de Vries]

On 20-05-19 22:14, Chet Ramey wrote:
> On 5/17/19 10:59 AM, Tom de Vries wrote:
> 
>> Either way, I'm open for suggestions that make gdb call
>> rl_set_screen_size with legal parameters, and disable features like
>> horizontal scrolling to get unformatted output for the testsuite run.
> 
> Here's a patch that will prevent the huge values for the screen width from
> causing at least one issue with line_size:
> 
> *** ../readline-8.0-patched/display.c 2018-09-30 21:37:48.000000000 -0400
> --- display.c 2019-05-16 16:50:44.000000000 -0400
> ***************
> *** 604,607 ****
> --- 604,610 ----
>     register int n;
> 
> +   if (line_size <= _rl_screenwidth) /* XXX - for gdb */
> +     line_size = _rl_screenwidth + 1;
> +
>     if (invisible_line == 0)  /* initialize it */
>       {
> 
> You're still going to have to deal with some horizontal scrolling if the
> input line gets long enough.
> 

Hi Chet,

thanks for the patch.

I've tried it out (together with the assert mentioned earlier) and found
that indeed it fixes the assert for the reported scenario:
...
$ TERM=dumb ./gdb -q -ex "set width 0"
(gdb)
...
but I still ran into the assert by typing the command instead of using
"-ex":
...
$ TERM=dumb ./gdb -q
(gdb) set width 0
gdb: display.c:1214: rl_redisplay: Assertion `last_lmargin +
(_rl_screenwidth + visible_wrap_offset) <= line_size' failed.
Aborted (core dumped)
...

Using this additional bit:
...
@@ -528,6 +533,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;
...
I managed to fix the assert also in this scenario, and managed to run
the entire gdb testsuite without triggering the assert.

Is that a good code change?

Thanks,
- Tom