bug#32271: heap buffer overflow in regexp.c, line 286

[Jim Meyering]

On Fri, Jul 27, 2018 at 12:13 PM, Assaf Gordon <address@hidden> wrote:
> Hello,
>
> On 25/07/18 08:34 AM, project-repo wrote:
>>
>> I let the fuzzer run again and it came up with a second heap buffer
>> overflow. This time in regexp.c, line 286. Here is a backtrace as
>> supplied by the address sanitizer:
>>
>> =================================================================
>> ==7428==ERROR: AddressSanitizer: heap-buffer-overflow on address
>> 0x611000000b2f at pc 0x7fee3354c574 bp 0x7ffd9adf2120 sp 0x7ffd9adf18d0
>> READ of size 238 at 0x611000000b2f thread T0
>>      #0 0x7fee3354c573  (/usr/lib/x86_64-linux-gnu/libasan.so.4+0x40573)
>>      #1 0x55aabd6d7025 in match_regex sed/regexp.c:286
>>      #2 0x55aabd6cd5a5 in do_subst sed/execute.c:1098
>>      #3 0x55aabd6cd5a5 in execute_program sed/execute.c:1507
>>      #4 0x55aabd6d4d5a in process_files sed/execute.c:1677
>>      #5 0x55aabd6ac5a2 in main sed/sed.c:377
>>      #6 0x7fee33173a86 in __libc_start_main
>> (/lib/x86_64-linux-gnu/libc.so.6+0x21a86)
>>      #7 0x55aabd6ad1c9 in _start (/home/jefeus/sed/sed/sed+0xc1c9)
>
>
> Once again, great work - you indirectly found a 15-year-old bug,
> in addition to the above heap buffer overflow.
>
> The two attached patches should explain it in detail.
>
> As these changes are somewhat subtle, I encourage everyone to
> double-check them...

Fine work, yet again. Thank you!
I did spot one nit: the addition of two leading TAB bytes in the
latter patch. Should be 8 spaces, of course:

-        str_append(&s_accum, line.active + start, offset - start);$
+^I{$
+          str_append(&s_accum, line.active + start, offset - start);$
+          start = offset;$
+^I}$
$

And in the added test, an even smaller comment nit: add an "s" after file:
- will report "binary file differ"
+ will report "binary files differ"