GNU bug report logs - #26690
Crash with --ignore-matching-lines

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26690 in the body.
You can then email your comments to 26690 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Marcel Böhme <marcel.boehme <at> nus.edu.sg>
To: "bug-diffutils <at> gnu.org" <bug-diffutils <at> gnu.org>
Subject: Crash with --ignore-matching-lines
Date: Fri, 28 Apr 2017 00:25:34 +0000

Dear all,

We found a null pointer dereference resulting in a segmentation fault for Diffutils. We think, it is an incorrect use of GLIBC but it can be easily addressed in both Diffutils and GLIBC. So we’ll make another bug report upstream. Specifically, The patch can be in GLIBC (introducing a simple null pointer check) or in Diffutils (preventing the NullDeref altogether). Below is a more detailed analysis.

This bug was found with AFLGo, a directed version of AFL/AFLFast. Thanks also to Van-Thuan Pham.

How to reproduce:
$ diff -Ia -I\\ <(printf "") <(echo a)
diff: \: Trailing backslash
diff: stack overflow

ASAN says:
ASAN:DEADLYSIGNAL
=================================================================
==74668==ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000d8 (pc 0x7f0670589bad bp 0x000000000000 sp 0x7ffefbed15b0 T0)
    #0 0x7f0670589bac in re_search_stub /build/eglibc-MjiXCM/eglibc-2.19/posix/regexec.c:414
    #1 0x7f067058a527 in re_search /build/eglibc-MjiXCM/eglibc-2.19/posix/regexec.c:312
    #2 0x555bfc in analyze_hunk /home/ubuntu/diffutils-analysis/diffutils/obj-asan/src/../../src/util.c:1522:8
    #3 0x4f91dd in diff_2_files /home/ubuntu/diffutils-analysis/diffutils/obj-asan/src/../../src/analyze.c:620:12
    #4 0x528971 in compare_files /home/ubuntu/diffutils-analysis/diffutils/obj-asan/src/../../src/diff.c:1434:11
    #5 0x51882c in main /home/ubuntu/diffutils-analysis/diffutils/obj-asan/src/../../src/diff.c:800:18
    #6 0x7f06704c4f44 in __libc_start_main /build/eglibc-MjiXCM/eglibc-2.19/csu/libc-start.c:287
    #7 0x41bac5 in _start (/home/ubuntu/diffutils-analysis/diffutils/obj-asan/src/diff+0x41bac5)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /build/eglibc-MjiXCM/eglibc-2.19/posix/regexec.c:414 in re_search_stub

This is our analysis:
For the diff-tool the argument -I<regexp> specifies the changed lines to exclude. For each such argument, the function add_regexp in diff.c is called. This function uses re_compile_pattern to successfully compile the first pattern. However, it fails to compile the second pattern, giving the error “Trailing backslash”. In both cases, the function uses the re_pattern_buffer *ignore_regexp. However, the failed compilation corrupts *ignore_regexp, setting ignore_regexp->buffer=0x0 and ignore_regexp->allocated=0. Later, in function summarize_regexp_list, it is established that at least one pattern was successfully compiled and ignore_regexp->fastmap is set, indicating that re_search is being called in utils.c:1501. Unfortunately, it is being called on the corrupted ignore_regexp where ignore_regexp->buf = 0x0. GLIBC does not check for a null-pointer when derefencing the buffer in regexec.c:413.

GDB says:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7af5056 in re_search_stub (bufp=0x6228a0 <ignore_regexp>, string=string <at> entry=0x62a050 "a\n", length=1, start=start <at> entry=0, range=1, stop=1, regs=0x0, ret_len=0) at regexec.c:413
413     in regexec.c
(gdb) p *bufp
$1 = {buffer = 0x0, allocated = 0, used = 224, syntax = 330310, fastmap = 0x6271f0 "\330\036\335\367\377\177", translate = 0x0, re_nsub = 0, can_be_null = 0, regs_allocated = 0, fastmap_accurate = 0, no_sub = 0, not_bol = 0, not_eol = 0, newline_anchor = 1}
(gdb) bt
#0  0x00007ffff7af5056 in re_search_stub (bufp=0x6228a0 <ignore_regexp>, string=string <at> entry=0x62a050 "a\n", length=1, start=start <at> entry=0, range=1, stop=1, regs=0x0, ret_len=0) at regexec.c:413
#1  0x00007ffff7af5a70 in __re_search (bufp=<optimized out>, string=string <at> entry=0x62a050 "a\n", length=<optimized out>, start=start <at> entry=0, range=<optimized out>, regs=regs <at> entry=0x0) at regexec.c:317
#2  0x000000000040ce1e in analyze_hunk (hunk=hunk <at> entry=0x627340, first0=first0 <at> entry=0x7fffffffdf80, last0=last0 <at> entry=0x7fffffffdf88, first1=first1 <at> entry=0x7fffffffdf90, last1=last1 <at> entry=0x7fffffffdf98) at util.c:1522
#3  0x000000000040507d in diff_2_files (cmp=cmp <at> entry=0x7fffffffe060) at analyze.c:620
#4  0x00000000004071f7 in compare_files (parent=parent <at> entry=0x0, name0=0x7fffffffe6ec "/dev/fd/63", name1=<optimized out>) at diff.c:1434
#5  0x000000000040387e in main (argc=<optimized out>, argv=<optimized out>) at diff.c:800

VALGRIND says:
==103798== Memcheck, a memory error detector
==103798== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==103798== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==103798== Command: src/diff -Ia -I\\ /dev/fd/63 /dev/fd/62
==103798==
src/diff: \: Trailing backslash
==103798== Invalid read of size 4
==103798==    at 0x4F21056: re_search_stub (regexec.c:413)
==103798==    by 0x4F21A6F: re_search (regexec.c:317)
==103798==    by 0x40CE1D: analyze_hunk (util.c:1522)
==103798==    by 0x40507C: diff_2_files (analyze.c:620)
==103798==    by 0x4071F6: compare_files (diff.c:1434)
==103798==    by 0x40387D: main (diff.c:800)
==103798==  Address 0xd8 is not stack'd, malloc'd or (recently) free'd
==103798==
diff: stack overflow
==103798==
==103798== HEAP SUMMARY:
==103798==     in use at exit: 4,970 bytes in 25 blocks
==103798==   total heap usage: 75 allocs, 50 frees, 28,030 bytes allocated
==103798==
==103798== LEAK SUMMARY:
==103798==    definitely lost: 136 bytes in 5 blocks
==103798==    indirectly lost: 120 bytes in 6 blocks
==103798==      possibly lost: 0 bytes in 0 blocks
==103798==    still reachable: 4,714 bytes in 14 blocks
==103798==         suppressed: 0 bytes in 0 blocks
==103798== Rerun with --leak-check=full to see details of leaked memory
==103798==
==103798== For counts of detected and suppressed errors, rerun with: -v
==103798== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Best regards,
- Marcel

---
Marcel Böhme
Senior Research Fellow
TSUNAMi Security Research Centre
National University of Singapore

________________________________

Important: This email is confidential and may be privileged. If you are not the intended recipient, please delete it and notify us immediately; you should not copy or use it for any purpose, nor disclose its contents to any other person. Thank you.

Message #8 received at 26690 <at> debbugs.gnu.org (full text, mbox):

From: Marcel Böhme <marcel.boehme <at> nus.edu.sg>
To: "26690 <at> debbugs.gnu.org" <26690 <at> debbugs.gnu.org>
Subject: Re: bug#26690: Acknowledgement (Crash with --ignore-matching-lines)
Date: Fri, 28 Apr 2017 01:32:30 +0000

Suggested patch:

%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
diff --git a/src/diff.c b/src/diff.c
index 76851ac..9db7005 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -818,7 +818,18 @@ add_regexp (struct regexp_list *reglist, char const *pattern)
  char const *m = re_compile_pattern (pattern, patlen, reglist->buf);

  if (m != 0)
-    error (0, 0, "%s: %s", pattern, m);
+    {
+      error (0, 0, "%s: %s", pattern, m);
+
+      /* Recompile existing pattern if needed */
+      if (reglist->regexps)
+       {
+         m = re_compile_pattern (reglist->regexps, reglist->len,
+                                 reglist->buf);
+         if (m)
+           die (EXIT_TROUBLE, 0, "%s: %s", reglist->regexps, m);
+       }
+    }
  else
    {
      char *regexps = reglist->regexps;
%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%

Best regards,
- Marcel

---
Marcel Böhme
Senior Research Fellow
TSUNAMi Security Research Centre
National University of Singapore


________________________________

Important: This email is confidential and may be privileged. If you are not the intended recipient, please delete it and notify us immediately; you should not copy or use it for any purpose, nor disclose its contents to any other person. Thank you.

Message #11 received at 26690 <at> debbugs.gnu.org (full text, mbox):

From: Marcel Böhme <marcel.boehme <at> nus.edu.sg>
To: "26690 <at> debbugs.gnu.org" <26690 <at> debbugs.gnu.org>
Subject: Re: bug#26690: Acknowledgement (Crash with --ignore-matching-lines)
Date: Fri, 28 Apr 2017 01:36:13 +0000

[Message part 1 (text/html, inline)]

Message #14 received at 26690 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Marcel Böhme <marcel.boehme <at> nus.edu.sg>,
 "26690-done <at> debbugs.gnu.org" <26690 <at> debbugs.gnu.org>
Subject: Re: [bug-diffutils] bug#26690: Acknowledgement (Crash with
 --ignore-matching-lines)
Date: Thu, 27 Apr 2017 22:43:05 -0700

[Message part 1 (text/plain, inline)]

Marcel Böhme wrote:
> Suggested patch:

Thanks, but I'm not seeing the point of continuing after an invalid regular 
expression is found. I installed the attached simpler patch for now.

[0001-diff-don-t-crash-if-regexp-is-invalid.txt (text/plain, attachment)]

Message #17 received at 26690 <at> debbugs.gnu.org (full text, mbox):

From: Marcel Böhme <marcel.boehme <at> nus.edu.sg>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: "26690-done <at> debbugs.gnu.org" <26690 <at> debbugs.gnu.org>
Subject: Re: [bug-diffutils] bug#26690: Acknowledgement (Crash with
 --ignore-matching-lines)
Date: Fri, 28 Apr 2017 05:53:59 +0000

LGTM

________________________________

Important: This email is confidential and may be privileged. If you are not the intended recipient, please delete it and notify us immediately; you should not copy or use it for any purpose, nor disclose its contents to any other person. Thank you.

Message #22 received at 26690-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Marcel Böhme <boehme.marcel <at> gmail.com>, 
 26690-done <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: Crash with --ignore-matching-lines
Date: Tue, 2 May 2017 20:12:25 -0700

I've just added a test and NEWS entry for that fix:

  https://git.savannah.gnu.org/cgit/diffutils.git/commit/?id=4e40686e23d

and also updated gnulib to the latest.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.