Commit 468e3ddc344d6374a615fb2199005956ad0eb531

Patrick Steinhardt 2019-10-19T16:48:11

patch_parse: fix out-of-bounds read with No-NL lines We've got two locations where we copy lines into the patch. The first one is when copying normal " ", "-" or "+" lines, while the second location gets executed when we copy "\ No newline at end of file" lines. While the first one correctly uses `git__strndup` to copy only until the newline, the other one doesn't. Thus, if the line occurs at the end of the patch and if there is no terminating NUL character, then it may result in an out-of-bounds read. Fix the issue by using `git__strndup`, as was already done in the other location. Furthermore, add allocation checks to both locations to detect out-of-memory situations.

diff --git a/src/patch_parse.c b/src/patch_parse.c
index 16f2f68..7a209fc 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -647,6 +647,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;
@@ -686,8 +687,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;
diff --git a/tests/patch/parse.c b/tests/patch/parse.c
index ede5a96..77a6dd6 100644
--- a/tests/patch/parse.c
+++ b/tests/patch/parse.c
@@ -161,3 +161,16 @@ 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);
+}