diff options
author | Patrick Steinhardt <ps@pks.im> | 2019-10-19 16:48:11 +0200 |
---|---|---|
committer | Patrick Steinhardt <ps@pks.im> | 2019-10-19 17:11:48 +0200 |
commit | 468e3ddc344d6374a615fb2199005956ad0eb531 (patch) | |
tree | 3e1f095c6bc4de792e9e31f24a9727fee6ba24b3 | |
parent | 6c6c15e935091a33f83d8de4ee5b0640339b2b89 (diff) | |
download | libgit2-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.c | 4 | ||||
-rw-r--r-- | tests/patch/parse.c | 13 |
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); +} |