[PATCH] diff-no-index: fix stdin path in subdirectory

[Gregoire Geis <opensource@gregoirege.is>]

`git diff --no-index` tried to normalize paths before printing them,
skipping a prefix before reading a string. However, `-` doesn't start
with this prefix, leading to a buffer overflow:

$ make SANITIZE=address
$ mkdir a && cd a
$ echo a | ../git diff --no-index /dev/null -
=================================================================
==14536==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60b000001f92 at pc 0x000102612a30 bp 0x00016db386d0 sp 0x00016db386c8
READ of size 1 at 0x60b000001f92 thread T0
    #0 0x102612a2c in diff_flush_patch diff.c:6154
    #1 0x10260dbfc in diff_flush diff.c:6829
    #2 0x1025f218c in diff_no_index diff-no-index.c:422
    #3 0x102356338 in cmd_diff diff.c:501
    #4 0x1022cc780 in run_builtin git.c:480
    #5 0x1022c9bec in handle_builtin git.c:746
    #6 0x1022c8670 in cmd_main git.c:953
    #7 0x1024fe230 in main common-main.c:9

Signed-off-by: Gregoire Geis <opensource@gregoirege.is>
---
Hello!

I noticed that running `git diff --no-index /dev/null -` in a
subdirectory leads to garbage file names (instead of `a/-` and `b/-`).

Valgrind and ASAN confirmed that this is a heap buffer overflow. I
fixed it by checking against "-" when writing the file names in the
patch, which leads to results similar to using an absolute path (e.g.
`a/dev/stdin`, `a/-`, no matter the working directory). "-" is a
special input for `git diff --no-index`, so I don't think we need any
kind of normalization before this check.

I added a test which currently fails on `master`. I chose a
subdirectory `aa/bb/cc` to make sure that the path is long enough for
garbage to appear even if git is compiled without ASAN.

 diff.c                        |  4 ++--
 t/meson.build                 |  1 +
 t/t4072-diff-no-index-dash.sh | 17 +++++++++++++++++
 3 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100755 t/t4072-diff-no-index-dash.sh

diff --git a/diff.c b/diff.c
index dca87e1..88e93e5 100644
--- a/diff.c
+++ b/diff.c
@@ -4626,12 +4626,12 @@ static void diff_fill_oid_info(struct diff_filespec *one, struct index_state *is
 static void strip_prefix(int prefix_length, const char **namep, const char **otherp)
 {
 	/* Strip the prefix but do not molest /dev/null and absolute paths */
-	if (*namep && !is_absolute_path(*namep)) {
+	if (*namep && !is_absolute_path(*namep) && strcmp(*namep, "-")) {
 		*namep += prefix_length;
 		if (**namep == '/')
 			++*namep;
 	}
-	if (*otherp && !is_absolute_path(*otherp)) {
+	if (*otherp && !is_absolute_path(*otherp) && strcmp(*otherp, "-")) {
 		*otherp += prefix_length;
 		if (**otherp == '/')
 			++*otherp;
diff --git a/t/meson.build b/t/meson.build
index bbeba1a..9042f3f 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -486,6 +486,7 @@ integration_tests = [
   't4069-remerge-diff.sh',
   't4070-diff-pairs.sh',
   't4071-diff-minimal.sh',
+  't4072-diff-no-index-dash.sh',
   't4100-apply-stat.sh',
   't4101-apply-nonl.sh',
   't4102-apply-rename.sh',
diff --git a/t/t4072-diff-no-index-dash.sh b/t/t4072-diff-no-index-dash.sh
new file mode 100755
index 0000000..35170a4
--- /dev/null
+++ b/t/t4072-diff-no-index-dash.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+test_description='`git diff file -` does not crash'
+
+. ./test-lib.sh
+
+test_expect_success '`git diff --no-index /dev/null -` works in root' '
+  echo foo | git diff --no-index /dev/null - | grep "a/- b/-"
+'
+
+test_expect_success '`git diff --no-index /dev/null -` works in subdirectory' '
+  mkdir -p aa/bb/cc &&
+  cd aa/bb/cc &&
+  echo foo | git diff --no-index /dev/null - | grep "a/- b/-"
+'
+
+test_done

base-commit: 64cbe5e2e8a7b0f92c780b210e602496bd5cad0f
-- 
2.48.1