Skip to content

Commit 468e3dd

Browse files
committed
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.
1 parent 6c6c15e commit 468e3dd

File tree

2 files changed

+16
-1
lines changed

2 files changed

+16
-1
lines changed

src/patch_parse.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,7 @@ static int parse_hunk_body(
647647

648648
line->content_len = ctx->parse_ctx.line_len - prefix;
649649
line->content = git__strndup(ctx->parse_ctx.line + prefix, line->content_len);
650+
GIT_ERROR_CHECK_ALLOC(line->content);
650651
line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
651652
line->origin = origin;
652653
line->num_lines = 1;
@@ -686,8 +687,9 @@ static int parse_hunk_body(
686687

687688
memset(line, 0x0, sizeof(git_diff_line));
688689

689-
line->content = git__strdup(ctx->parse_ctx.line);
690690
line->content_len = ctx->parse_ctx.line_len;
691+
line->content = git__strndup(ctx->parse_ctx.line, line->content_len);
692+
GIT_ERROR_CHECK_ALLOC(line->content);
691693
line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
692694
line->origin = eof_for_origin(last_origin);
693695
line->num_lines = 1;

tests/patch/parse.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,16 @@ void test_patch_parse__memory_leak_on_multiple_paths(void)
161161
git_patch *patch;
162162
cl_git_fail(git_patch_from_buffer(&patch, PATCH_MULTIPLE_OLD_PATHS, strlen(PATCH_MULTIPLE_OLD_PATHS), NULL));
163163
}
164+
165+
void test_patch_parse__truncated_no_newline_at_end_of_file(void)
166+
{
167+
size_t len = strlen(PATCH_APPEND_NO_NL) - strlen("at end of file\n");
168+
const git_diff_line *line;
169+
git_patch *patch;
170+
171+
cl_git_pass(git_patch_from_buffer(&patch, PATCH_APPEND_NO_NL, len, NULL));
172+
cl_git_pass(git_patch_get_line_in_hunk(&line, patch, 0, 4));
173+
cl_assert_equal_s(line->content, "\\ No newline ");
174+
175+
git_patch_free(patch);
176+
}

0 commit comments

Comments
 (0)