Subject: Re: [bug-diffutils] bug#31935: bug#31935: 2 crashes in diffutills commit version 576645c

From: Paul Eggert <[email-masked]>
Date: Fri, 28 Dec 2018 19:11:02 -0800

Jim Meyering wrote:
> There are still numerous unguarded [-1] references, so this updated
> patch is doubtless still incomplete:

The real bug was elsewhere, I think. I installed the attached patch. This patch 
lacks your test case, which didn't work for me because there is no 
require_valgrind_ in diffutils. Is require_valgrind_ from coreutils or from some 
other location?
>From c0097514df398ab70c58fe75af17ba0d7ec58c73 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Fri, 28 Dec 2018 19:00:50 -0800
Subject: [PATCH] diff: fix UMR with --strip-trailing-cr

Problem reported by Hongxu Chen (Bug#31935).
* src/io.c (prepare_text): Strip trailing CR before
doing the rest of the analysis.
* NEWS: Mention the fix.
Co-authored-by: Jim Meyering <[email-masked]>
---
 NEWS     |  7 +++++++
 src/io.c | 39 +++++++++++++++------------------------
 2 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/NEWS b/NEWS
index 56e0445..7d115a3 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,13 @@ GNU diffutils NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug fixes
+
+  diff --strip-trailing-cr with a single CR byte in one input file
+  would provoke an uninitialized memory read, e.g.,
+    diff -a --strip-trailing-cr <(printf '\r') <(echo a)
+  [bug introduced in 2.8 with addition of the --strip-trailing-cr option]
+
 
 * Noteworthy changes in release 3.6 (2017-05-21) [stable]
 
diff --git a/src/io.c b/src/io.c
index fb86392..6c03c70 100644
--- a/src/io.c
+++ b/src/io.c
@@ -481,42 +481,33 @@ prepare_text (struct file_data *current)
 {
   size_t buffered = current->buffered;
   char *p = FILE_BUFFER (current);
-
-  if (buffered == 0 || p[buffered - 1] == '\n')
-    current->missing_newline = false;
-  else
-    {
-      p[buffered++] = '\n';
-      current->missing_newline = true;
-    }
-
   if (!p)
     return;
 
-  /* Don't use uninitialized storage when planting or using sentinels.  */
-  memset (p + buffered, 0, sizeof (word));
-
   if (strip_trailing_cr)
     {
-      char *dst;
       char *srclim = p + buffered;
       *srclim = '\r';
-      dst = rawmemchr (p, '\r');
+      char *dst = rawmemchr (p, '\r');
 
-      if (dst != srclim)
+      for (char const *src = dst; src != srclim; src++)
 	{
-	  char const *src = dst;
-	  do
-	    {
-	      *dst = *src++;
-	      dst += ! (*dst == '\r' && *src == '\n');
-	    }
-	  while (src < srclim);
-
-	  buffered -= src - dst;
+	  src += *src == '\r' && src[1] == '\n';
+	  *dst++ = *src;
 	}
+
+      buffered -= srclim - dst;
     }
 
+  if (buffered != 0 && p[buffered - 1] != '\n')
+    {
+      p[buffered++] = '\n';
+      current->missing_newline = true;
+    }
+
+  /* Don't use uninitialized storage when planting or using sentinels.  */
+  memset (p + buffered, 0, sizeof (word));
+
   current->buffered = buffered;
 }
 
-- 
2.17.1