Subject: bug#23983: closed (Re: bug#23983: [PATCH] grep: fix crash with a pattern of alternation of two same characters)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 14 Jul 2016 18:01:02 +0000

Your bug report

#23983: [PATCH] grep: fix crash with a pattern of alternation of two same characters

which was filed against the grep package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
23983: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23983
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#23983: [PATCH] grep: fix crash with a pattern of alternation of two same characters
From: Paul Eggert <[email-masked]>
To: Eric Blake <[email-masked]>, Norihiro Tanaka <[email-masked]>, [email-masked]
Date: Thu, 14 Jul 2016 20:00:35 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

On 07/14/2016 07:32 PM, Eric Blake wrote:
> Could you instead do:
Something like that should work, but the original patch seems more 
straightforward. As it happens I installed the attached before seeing 
your email. The first is the original patch but with comments spruced up 
a bit, mostly for English. The second is a minor style change.
>From 96daaf5b8df02171f609b17edbf1ece17edc2736 Mon Sep 17 00:00:00 2001
From: Norihiro Tanaka <[email-masked]>
Date: Thu, 14 Jul 2016 23:45:45 +0900
Subject: [PATCH 1/2] grep: fix -F crash when alternating duplicates

grep -F crashes with a pattern like 0\n0.
This bug was introduced in 966f6586fbce3081ce6e5e2f9b55301b0ec3d2b4.

* src/kwset.c (memoff2_kwset): If two characters are the same,
use memchr instead of memchr2.
* tests/two-chars: New test.
* tests/Makefile.am (TESTS): Add it.
---
 src/kwset.c       |  7 ++++++-
 tests/Makefile.am |  1 +
 tests/two-chars   | 24 ++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100755 tests/two-chars

diff --git a/src/kwset.c b/src/kwset.c
index 7391990..80df752 100644
--- a/src/kwset.c
+++ b/src/kwset.c
@@ -643,8 +643,13 @@ memoff2_kwset (char const *s, size_t n, kwset_t kwset,
 {
   struct tree const *link = kwset->trie->links;
   struct tree const *clink = link->llink ? link->llink : link->rlink;
+  char const *mch;
+
+  if (clink)
+    mch = memchr2 (s, link->label, clink->label, n);
+  else
+    mch = memchr (s, link->label, n);
 
-  char const *mch = memchr2 (s, link->label, clink->label, n);
   if (! mch)
     return SIZE_MAX;
   else
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 38b0e32..b29328b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -134,6 +134,7 @@ TESTS =						\
   turkish-I					\
   turkish-I-without-dot				\
   turkish-eyes					\
+  two-chars					\
   two-files					\
   unibyte-binary				\
   unibyte-bracket-expr				\
diff --git a/tests/two-chars b/tests/two-chars
new file mode 100755
index 0000000..7ce7216
--- /dev/null
+++ b/tests/two-chars
@@ -0,0 +1,24 @@
+#! /bin/sh
+# Check for grep -F with two patterns consisting of the same char.
+#
+# Copyright 2016 Free Software Foundation, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+require_en_utf8_locale_
+
+fail=0
+
+for LOC in en_US.UTF-8 $zh $LOCALE_FR_UTF8; do
+  printf '0\n0\n' >pat
+  printf '0\n' >in
+  out=out-$LOC
+  LC_ALL=$LOC grep -Ff pat in >$out || fail=1
+  compare in $out || fail=1
+done
+
+Exit $fail
-- 
2.5.5

>From 01f728d0e10ad6483ac785d1963e46b544db5948 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Thu, 14 Jul 2016 19:38:01 +0200
Subject: [PATCH 2/2] grep: minor style changes for -F crash fix

* src/kwset.c (memoff2_kwset): Use ?: instead of if-else.
---
 src/kwset.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/kwset.c b/src/kwset.c
index 80df752..264ef22 100644
--- a/src/kwset.c
+++ b/src/kwset.c
@@ -643,13 +643,9 @@ memoff2_kwset (char const *s, size_t n, kwset_t kwset,
 {
   struct tree const *link = kwset->trie->links;
   struct tree const *clink = link->llink ? link->llink : link->rlink;
-  char const *mch;
-
-  if (clink)
-    mch = memchr2 (s, link->label, clink->label, n);
-  else
-    mch = memchr (s, link->label, n);
-
+  char const *mch = (clink
+                     ? memchr2 (s, link->label, clink->label, n)
+                     : memchr (s, link->label, n));
   if (! mch)
     return SIZE_MAX;
   else
-- 
2.5.5

Subject: [PATCH] grep: fix crash with a pattern of alternation of two same characters
From: Norihiro Tanaka <[email-masked]>
To: <[email-masked]>
Date: Fri, 15 Jul 2016 00:07:13 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

Current master crashes with below.

$ printf '0\n0' >pat
$ printf '0\n' >in
$ env LC_ALL=C grep -F pat in

grep -F uses memchr2() for each character in this pattern, but if two
characters is same, the trie has no child.
From 3b004a66cc9dc66ad62111bc38b2c5a2b8bb082c Mon Sep 17 00:00:00 2001
From: Norihiro Tanaka <[email-masked]>
Date: Thu, 14 Jul 2016 23:45:45 +0900
Subject: [PATCH] grep: fix crash with a pattern of alternation of two same characters

grep -F crashes with pattern as 0\n0.  This bug is introduced in
966f6586fbce3081ce6e5e2f9b55301b0ec3d2b4.

* src/kwset.c (memoff2_kwset): If two characters is same, memchr instead
of memchr2.
* tests/two-chars: New test.
* tests/Makefile.am (TESTS): Add it.
---
 src/kwset.c       |    7 ++++++-
 tests/Makefile.am |    1 +
 tests/two-chars   |   25 +++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 1 deletions(-)
 create mode 100755 tests/two-chars

diff --git a/src/kwset.c b/src/kwset.c
index 7391990..80df752 100644
--- a/src/kwset.c
+++ b/src/kwset.c
@@ -643,8 +643,13 @@ memoff2_kwset (char const *s, size_t n, kwset_t kwset,
 {
   struct tree const *link = kwset->trie->links;
   struct tree const *clink = link->llink ? link->llink : link->rlink;
+  char const *mch;
+
+  if (clink)
+    mch = memchr2 (s, link->label, clink->label, n);
+  else
+    mch = memchr (s, link->label, n);
 
-  char const *mch = memchr2 (s, link->label, clink->label, n);
   if (! mch)
     return SIZE_MAX;
   else
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 38b0e32..b29328b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -134,6 +134,7 @@ TESTS =						\
   turkish-I					\
   turkish-I-without-dot				\
   turkish-eyes					\
+  two-chars					\
   two-files					\
   unibyte-binary				\
   unibyte-bracket-expr				\
diff --git a/tests/two-chars b/tests/two-chars
new file mode 100755
index 0000000..43a1418
--- /dev/null
+++ b/tests/two-chars
@@ -0,0 +1,25 @@
+#! /bin/sh
+# Read two files, of increasing size.
+# With ASAN, this would have triggered a false-positive read of poisoned memory.
+#
+# Copyright (C) 2014-2016 Free Software Foundation, Inc.
+#
+# Copying and distribution of this file, with or without modification,
+# are permitted in any medium without royalty provided the copyright
+# notice and this notice are preserved.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+require_en_utf8_locale_
+
+fail=0
+
+for LOC in en_US.UTF-8 $zh $LOCALE_FR_UTF8; do
+  printf '0\n0\n' >pat
+  printf '0\n' >in
+  out=out-$LOC
+  LC_ALL=$LOC grep -Ff pat in >$out || fail=1
+  compare in $out || fail=1
+done
+
+Exit $fail
-- 
1.7.1