diff mbox

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

Message ID 17c0fa1d-7df4-5204-a8e6-104239d66e3c@suse.de
State New, archived
Headers

Commit Message

Tom de Vries May 23, 2019, 7:28 p.m. UTC 
  On 23-05-19 14:38, Chet Ramey wrote:
> On 5/23/19 3:33 AM, Tom de Vries wrote:
> 
>> 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?
> 
> It looks like it will solve that problem, and perhaps more. Thanks for the
> patch.

I did a further test-run to see if the original problem (PR24514 -
heap-buffer-overflow in update_line for utf8-identifiers.exp:
https://sourceware.org/bugzilla/show_bug.cgi?id=24514 ) was fixed, which
turned out not to be the case.

I've analyzed this, and found it to be caused by the
init_line_structures part of the patch changing line_size, which does
not force a reallloc. I've fixed this by changing minsize instead.

Attached patch passes gdb regression testsuite with the assert enabled,
and with gdb build with -fsanitize=address.

Thanks,
- Tom

Comments

Chet Ramey May 24, 2019, 1:08 p.m. UTC | #1 
On 5/23/19 3:28 PM, Tom de Vries wrote:

> I did a further test-run to see if the original problem (PR24514 -
> heap-buffer-overflow in update_line for utf8-identifiers.exp:
> https://sourceware.org/bugzilla/show_bug.cgi?id=24514 ) was fixed, which
> turned out not to be the case.
> 
> I've analyzed this, and found it to be caused by the
> init_line_structures part of the patch changing line_size, which does
> not force a reallloc. I've fixed this by changing minsize instead.

Thanks for the analysis and updated patch.

Chet
diff mbox

Patch

tentative

---
 readline/display.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/readline/display.c b/readline/display.c
index 712ca79b6d..f34ad05444 100644
--- a/readline/display.c
+++ b/readline/display.c
@@ -452,6 +452,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)
@@ -528,6 +531,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;