Merge pull request #5276 from pks-t/pks/patch-fuzzing-fixes

patch_parse: fixes for fuzzing errors

src/integer.h

@@ -72,15 +72,25 @@ GIT_INLINE(int) git__is_int(long long p)
 #  error compiler has add with overflow intrinsics but SIZE_MAX is unknown
 # endif
 
+# define git__add_int_overflow(out, one, two) \
+    __builtin_sadd_overflow(one, two, out)
+# define git__sub_int_overflow(out, one, two) \
+    __builtin_ssub_overflow(one, two, out)
+
 /* Use Microsoft's safe integer handling functions where available */
 #elif defined(_MSC_VER)
 
+# define ENABLE_INTSAFE_SIGNED_FUNCTIONS
 # include <intsafe.h>
 
 # define git__add_sizet_overflow(out, one, two) \
     (SizeTAdd(one, two, out) != S_OK)
 # define git__multiply_sizet_overflow(out, one, two) \
     (SizeTMult(one, two, out) != S_OK)
+#define git__add_int_overflow(out, one, two) \
+    (IntAdd(one, two, out) != S_OK)
+#define git__sub_int_overflow(out, one, two) \
+    (IntSub(one, two, out) != S_OK)
 
 #else
 
@@ -108,6 +118,24 @@ GIT_INLINE(bool) git__multiply_sizet_overflow(size_t *out, size_t one, size_t tw
 	return false;
 }
 
+GIT_INLINE(bool) git__add_int_overflow(int *out, int one, int two)
+{
+	if ((two > 0 && one > (INT_MAX - two)) ||
+	    (two < 0 && one < (INT_MIN - two)))
+		return true;
+	*out = one + two;
+	return false;
+}
+
+GIT_INLINE(bool) git__sub_int_overflow(int *out, int one, int two)
+{
+	if ((two > 0 && one < (INT_MIN + two)) ||
+	    (two < 0 && one > (INT_MAX + two)))
+		return true;
+	*out = one - two;
+	return false;
+}
+
 #endif
 
 #endif

src/patch_parse.c

@@ -69,6 +69,10 @@ static int parse_header_path_buf(git_buf *path, git_patch_parse_ctx *ctx, size_t
 {
 	int error;
 
+	if (!path_len)
+		return git_parse_err("patch contains empty path at line %"PRIuZ,
+				     ctx->parse_ctx.line_num);
+
 	if ((error = git_buf_put(path, ctx->parse_ctx.line, path_len)) < 0)
 		goto done;
 
@@ -91,10 +95,14 @@ done:
 static int parse_header_path(char **out, git_patch_parse_ctx *ctx)
 {
 	git_buf path = GIT_BUF_INIT;
-	int error = parse_header_path_buf(&path, ctx, header_path_len(ctx));
+	int error;
 
+	if ((error = parse_header_path_buf(&path, ctx, header_path_len(ctx))) < 0)
+		goto out;
 	*out = git_buf_detach(&path);
 
+out:
+	git_buf_dispose(&path);
 	return error;
 }
 
@@ -104,6 +112,12 @@ static int parse_header_git_oldpath(
 	git_buf old_path = GIT_BUF_INIT;
 	int error;
 
+	if (patch->old_path) {
+		error = git_parse_err("patch contains duplicate old path at line %"PRIuZ,
+				      ctx->parse_ctx.line_num);
+		goto out;
+	}
+
 	if ((error = parse_header_path_buf(&old_path, ctx, ctx->parse_ctx.line_len - 1)) <  0)
 		goto out;
 
@@ -120,9 +134,14 @@ static int parse_header_git_newpath(
 	git_buf new_path = GIT_BUF_INIT;
 	int error;
 
-	if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) <  0)
+	if (patch->new_path) {
+		error = git_parse_err("patch contains duplicate new path at line %"PRIuZ,
+				      ctx->parse_ctx.line_num);
 		goto out;
+	}
 
+	if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) <  0)
+		goto out;
 	patch->new_path = git_buf_detach(&new_path);
 
 out:
@@ -564,11 +583,17 @@ static int parse_hunk_body(
 		!git_parse_ctx_contains_s(&ctx->parse_ctx, "@@ -");
 		git_parse_advance_line(&ctx->parse_ctx)) {
 
+		int old_lineno, new_lineno, origin, prefix = 1;
 		char c;
-		int origin;
-		int prefix = 1;
-		int old_lineno = hunk->hunk.old_start + (hunk->hunk.old_lines - oldlines);
-		int new_lineno = hunk->hunk.new_start + (hunk->hunk.new_lines - newlines);
+
+		if (git__add_int_overflow(&old_lineno, hunk->hunk.old_start, hunk->hunk.old_lines) ||
+		    git__sub_int_overflow(&old_lineno, old_lineno, oldlines) ||
+		    git__add_int_overflow(&new_lineno, hunk->hunk.new_start, hunk->hunk.new_lines) ||
+		    git__sub_int_overflow(&new_lineno, new_lineno, newlines)) {
+			error = git_parse_err("unrepresentable line count at line %"PRIuZ,
+					      ctx->parse_ctx.line_num);
+			goto done;
+		}
 
 		if (ctx->parse_ctx.line_len == 0 || ctx->parse_ctx.line[ctx->parse_ctx.line_len - 1] != '\n') {
 			error = git_parse_err("invalid patch instruction at line %"PRIuZ,
@@ -628,6 +653,7 @@ static int parse_hunk_body(
 
 		line->content_len = ctx->parse_ctx.line_len - prefix;
 		line->content = git__strndup(ctx->parse_ctx.line + prefix, line->content_len);
+		GIT_ERROR_CHECK_ALLOC(line->content);
 		line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
 		line->origin = origin;
 		line->num_lines = 1;
@@ -667,8 +693,9 @@ static int parse_hunk_body(
 
 		memset(line, 0x0, sizeof(git_diff_line));
 
-		line->content = git__strdup(ctx->parse_ctx.line);
 		line->content_len = ctx->parse_ctx.line_len;
+		line->content = git__strndup(ctx->parse_ctx.line, line->content_len);
+		GIT_ERROR_CHECK_ALLOC(line->content);
 		line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
 		line->origin = eof_for_origin(last_origin);
 		line->num_lines = 1;

tests/patch/parse.c

@@ -148,3 +148,36 @@ void test_patch_parse__lifetime_of_patch_does_not_depend_on_buffer(void)
 
 	git_patch_free(patch);
 }
+
+void test_patch_parse__binary_file_with_missing_paths(void)
+{
+	git_patch *patch;
+	cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_WITH_MISSING_PATHS,
+					  strlen(PATCH_BINARY_FILE_WITH_MISSING_PATHS), NULL));
+}
+
+void test_patch_parse__memory_leak_on_multiple_paths(void)
+{
+	git_patch *patch;
+	cl_git_fail(git_patch_from_buffer(&patch, PATCH_MULTIPLE_OLD_PATHS, strlen(PATCH_MULTIPLE_OLD_PATHS), NULL));
+}
+
+void test_patch_parse__truncated_no_newline_at_end_of_file(void)
+{
+	size_t len = strlen(PATCH_APPEND_NO_NL) - strlen("at end of file\n");
+	const git_diff_line *line;
+	git_patch *patch;
+
+	cl_git_pass(git_patch_from_buffer(&patch, PATCH_APPEND_NO_NL, len, NULL));
+	cl_git_pass(git_patch_get_line_in_hunk(&line, patch, 0, 4));
+	cl_assert_equal_s(line->content, "\\ No newline ");
+
+	git_patch_free(patch);
+}
+
+void test_patch_parse__line_number_overflow(void)
+{
+	git_patch *patch;
+	cl_git_fail(git_patch_from_buffer(&patch, PATCH_INTMAX_NEW_LINES, strlen(PATCH_INTMAX_NEW_LINES), NULL));
+	git_patch_free(patch);
+}

tests/patch/patch_common.h

@@ -905,3 +905,24 @@
 	"-b\n" \
 	"+bb\n" \
 	" c\n"
+
+#define PATCH_BINARY_FILE_WITH_MISSING_PATHS \
+	"diff --git  \n" \
+	"--- \n" \
+	"+++ \n" \
+	"Binary files "
+
+#define PATCH_MULTIPLE_OLD_PATHS \
+	"diff --git  \n" \
+	"---  \n" \
+	"+++ \n" \
+	"index 0000..7DDb\n" \
+	"--- \n"
+
+#define PATCH_INTMAX_NEW_LINES \
+	"diff --git a/file b/file\n" \
+	"--- a/file\n" \
+	"+++ b/file\n" \
+	"@@ -0 +2147483647 @@\n" \
+	"\n" \
+	"  "