summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <ps@pks.im>2019-10-19 16:48:11 +0200
committerPatrick Steinhardt <ps@pks.im>2019-10-19 17:11:48 +0200
commit468e3ddc344d6374a615fb2199005956ad0eb531 (patch)
tree3e1f095c6bc4de792e9e31f24a9727fee6ba24b3
parent6c6c15e935091a33f83d8de4ee5b0640339b2b89 (diff)
downloadlibgit2-468e3ddc344d6374a615fb2199005956ad0eb531.tar.gz
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.
-rw-r--r--src/patch_parse.c4
-rw-r--r--tests/patch/parse.c13
2 files changed, 16 insertions, 1 deletions
diff --git a/src/patch_parse.c b/src/patch_parse.c
index 16f2f6832..7a209fc80 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 ede5a96e2..77a6dd60d 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);
+}