Commit 608cb07d by Patrick Steinhardt

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.
parent 3223f5de
...@@ -635,6 +635,7 @@ static int parse_hunk_body( ...@@ -635,6 +635,7 @@ static int parse_hunk_body(
line->content_len = ctx->parse_ctx.line_len - prefix; line->content_len = ctx->parse_ctx.line_len - prefix;
line->content = git__strndup(ctx->parse_ctx.line + prefix, line->content_len); 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->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
line->origin = origin; line->origin = origin;
line->num_lines = 1; line->num_lines = 1;
...@@ -674,8 +675,9 @@ static int parse_hunk_body( ...@@ -674,8 +675,9 @@ static int parse_hunk_body(
memset(line, 0x0, sizeof(git_diff_line)); 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_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->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
line->origin = eof_for_origin(last_origin); line->origin = eof_for_origin(last_origin);
line->num_lines = 1; line->num_lines = 1;
......
...@@ -161,3 +161,16 @@ void test_patch_parse__memory_leak_on_multiple_paths(void) ...@@ -161,3 +161,16 @@ void test_patch_parse__memory_leak_on_multiple_paths(void)
git_patch *patch; git_patch *patch;
cl_git_fail(git_patch_from_buffer(&patch, PATCH_MULTIPLE_OLD_PATHS, strlen(PATCH_MULTIPLE_OLD_PATHS), NULL)); 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);
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment