From 4cfb709bd848afba6834895a69eb01d2bbfda736 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@fb.com>
Date: Fri, 28 Dec 2018 16:12:11 -0800
Subject: [PATCH] diff: avoid uninitialized memory read with
 --strip-trailing-cr

* NEWS: (Bug fixed): Mention it.
* src/util.c (print_1_line_nl): Avoid --strip-trailing-cr UMR when the
buffer consists of a sole CR. That happened due to an unchecked
reference to limit[-1]. Ensure limit[-1] is valid before accessing it.
* src/normal.c (print_normal_hunk): Likewise.
* tests/strip-trailing-cr: New file. Test for that bug.
* tests/Makefile.am (TESTS): Add it.
Reported by Hongxu Chen <leftcopy.chx@gmail.com> in
http://bugs.gnu.org/31935
---
 NEWS                    |  7 +++++++
 src/normal.c            |  4 +++-
 src/util.c              |  4 ++--
 tests/Makefile.am       |  1 +
 tests/strip-trailing-cr | 20 ++++++++++++++++++++
 5 files changed, 33 insertions(+), 3 deletions(-)
 create mode 100755 tests/strip-trailing-cr

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/normal.c b/src/normal.c
index c6aac07..575fbbe 100644
--- a/src/normal.c
+++ b/src/normal.c
@@ -66,7 +66,9 @@ print_normal_hunk (struct change *hunk)
           print_1_line_nl ("<", &files[0].linbuf[i], true);
           if (i == last0)
             set_color_context (RESET_CONTEXT);
-          if (files[0].linbuf[i + 1][-1] == '\n')
+          char const *base = files[0].linbuf[i];
+          char const *limit = files[0].linbuf[i + 1];
+          if (base < limit && limit[-1] == '\n')
             putc ('\n', outfile);
         }
     }
diff --git a/src/util.c b/src/util.c
index 4f4d9bb..e4cd0ae 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1246,9 +1246,9 @@ print_1_line_nl (char const *line_flag, char const *const *line, bool skip_nl)
       fprintf (out, flag_format_1, line_flag_1);
     }

-  output_1_line (base, limit - (skip_nl && limit[-1] == '\n'), flag_format, line_flag);
+  output_1_line (base, limit - (skip_nl && base < limit && limit[-1] == '\n'), flag_format, line_flag);

-  if ((!line_flag || line_flag[0]) && limit[-1] != '\n')
+  if ((!line_flag || line_flag[0]) && base < limit && limit[-1] != '\n')
     {
       set_color_context (RESET_CONTEXT);
       fprintf (out, "\n\\ %s\n", _("No newline at end of file"));
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 26646c0..b1fe321 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -21,6 +21,7 @@ TESTS = \
   stdin \
   strcoll-0-names \
   filename-quoting \
+  strip-trailing-cr \
   colors

 XFAIL_TESTS = large-subopt
diff --git a/tests/strip-trailing-cr b/tests/strip-trailing-cr
new file mode 100755
index 0000000..9ec96f1
--- /dev/null
+++ b/tests/strip-trailing-cr
@@ -0,0 +1,20 @@
+#!/bin/sh
+# Before diff-3.7, this would provoke a UMR
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+fail=0
+
+require_valgrind_
+
+printf '\r' > r || framework_failure_
+echo b > b || framework_failure_
+printf '%s\n' 1c1 '< ---' '> b' > exp || framework_failure_
+
+returns_ 1 valgrind --quiet --error-exitcode=3 \
+   diff -a --strip-trailing-cr r b > out 2> err || fail=1
+
+compare exp out || fail=1
+compare /dev/null err || fail=1
+
+Exit $fail
-- 
2.20.1.2.gb21ebb671b

