Re: [PATCH 2/2] apply: handle assertion failure gracefully

["René Scharfe" <l.s.r@web.de>]

Am 25.02.2017 um 11:13 schrieb Vegard Nossum:
> For the patches in the added testcases, we were crashing with:
> 
>      git-apply: apply.c:3665: check_preimage: Assertion `patch->is_new <= 0' failed.


> diff --git a/t/t4154-apply-git-header.sh b/t/t4154-apply-git-header.sh
> index d651af4a2..c440c48ad 100755
> --- a/t/t4154-apply-git-header.sh
> +++ b/t/t4154-apply-git-header.sh
> @@ -12,4 +12,40 @@ rename new 0
>   EOF
>   '
>   
> +test_expect_success 'apply deleted file mode / new file mode / wrong mode' '
> +	test_must_fail git apply << EOF
> +diff --git a/. b/.
> +deleted file mode
> +new file mode
> +EOF
> +'

-- >8 --
Subject: [PATCH] apply: check git diffs for invalid file modes

An empty string as mode specification is accepted silently by git apply,
as Vegard Nossum found out using AFL.  It's interpreted as zero.  Reject
such bogus file modes, and only accept ones consisting exclusively of
octal digits.

Reported-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 apply.c                   | 17 ++++++++++++-----
 t/t4129-apply-samemode.sh | 16 +++++++++++++++-
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/apply.c b/apply.c
index 8a5e44c474..db38bc3cdd 100644
--- a/apply.c
+++ b/apply.c
@@ -1001,20 +1001,27 @@ static int gitdiff_newname(struct apply_state *state,
 				   DIFF_NEW_NAME);
 }
 
+static int parse_mode_line(const char *line, int linenr, unsigned int *mode)
+{
+	char *end;
+	*mode = strtoul(line, &end, 8);
+	if (end == line || !isspace(*end))
+		return error(_("invalid mode on line %d: %s"), linenr, line);
+	return 0;
+}
+
 static int gitdiff_oldmode(struct apply_state *state,
 			   const char *line,
 			   struct patch *patch)
 {
-	patch->old_mode = strtoul(line, NULL, 8);
-	return 0;
+	return parse_mode_line(line, state->linenr, &patch->old_mode);
 }
 
 static int gitdiff_newmode(struct apply_state *state,
 			   const char *line,
 			   struct patch *patch)
 {
-	patch->new_mode = strtoul(line, NULL, 8);
-	return 0;
+	return parse_mode_line(line, state->linenr, &patch->new_mode);
 }
 
 static int gitdiff_delete(struct apply_state *state,
@@ -1128,7 +1135,7 @@ static int gitdiff_index(struct apply_state *state,
 	memcpy(patch->new_sha1_prefix, line, len);
 	patch->new_sha1_prefix[len] = 0;
 	if (*ptr == ' ')
-		patch->old_mode = strtoul(ptr+1, NULL, 8);
+		return gitdiff_oldmode(state, ptr + 1, patch);
 	return 0;
 }
 
diff --git a/t/t4129-apply-samemode.sh b/t/t4129-apply-samemode.sh
index c268298eaf..5cdd76dfa7 100755
--- a/t/t4129-apply-samemode.sh
+++ b/t/t4129-apply-samemode.sh
@@ -13,7 +13,9 @@ test_expect_success setup '
 	echo modified >file &&
 	git diff --stat -p >patch-0.txt &&
 	chmod +x file &&
-	git diff --stat -p >patch-1.txt
+	git diff --stat -p >patch-1.txt &&
+	sed "s/^\(new mode \).*/\1/" <patch-1.txt >patch-empty-mode.txt &&
+	sed "s/^\(new mode \).*/\1garbage/" <patch-1.txt >patch-bogus-mode.txt
 '
 
 test_expect_success FILEMODE 'same mode (no index)' '
@@ -59,4 +61,16 @@ test_expect_success FILEMODE 'mode update (index only)' '
 	git ls-files -s file | grep "^100755"
 '
 
+test_expect_success FILEMODE 'empty mode is rejected' '
+	git reset --hard &&
+	test_must_fail git apply patch-empty-mode.txt 2>err &&
+	test_i18ngrep "invalid mode" err
+'
+
+test_expect_success FILEMODE 'bogus mode is rejected' '
+	git reset --hard &&
+	test_must_fail git apply patch-bogus-mode.txt 2>err &&
+	test_i18ngrep "invalid mode" err
+'
+
 test_done
-- 
2.13.2