Commit de543e297d85f1ac700c2e5a83e70b1bd32b9518
2019-11-05T22:44:27
patch_parse: fix segfault when header path contains whitespace only When parsing header paths from a patch, we reject any patches with empty paths as malformed patches. We perform the check whether a path is empty before sanitizing it, though, which may lead to a path becoming empty after the check, e.g. if we have trimmed whitespace. This may lead to a segfault later when any part of our patching logic actually references such a path, which may then be a `NULL` pointer. Fix the issue by performing the check after sanitizing. Add tests to catch the issue as they would have produced a segfault previosuly.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89
diff --git a/src/patch_parse.c b/src/patch_parse.c
index 5032e35..1bf0190 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -69,27 +69,24 @@ static int parse_header_path_buf(git_buf *path, git_patch_parse_ctx *ctx, size_t
{
int error;
- if (!path_len)
- return git_parse_err("patch contains empty path at line %"PRIuZ,
- ctx->parse_ctx.line_num);
-
if ((error = git_buf_put(path, ctx->parse_ctx.line, path_len)) < 0)
- goto done;
+ return error;
git_parse_advance_chars(&ctx->parse_ctx, path_len);
git_buf_rtrim(path);
- if (path->size > 0 && path->ptr[0] == '"')
- error = git_buf_unquote(path);
-
- if (error < 0)
- goto done;
+ if (path->size > 0 && path->ptr[0] == '"' &&
+ (error = git_buf_unquote(path)) < 0)
+ return error;
git_path_squash_slashes(path);
-done:
- return error;
+ if (!path->size)
+ return git_parse_err("patch contains empty path at line %"PRIuZ,
+ ctx->parse_ctx.line_num);
+
+ return 0;
}
static int parse_header_path(char **out, git_patch_parse_ctx *ctx)
diff --git a/tests/patch/parse.c b/tests/patch/parse.c
index 9067f4a..c18b63a 100644
--- a/tests/patch/parse.c
+++ b/tests/patch/parse.c
@@ -156,6 +156,20 @@ void test_patch_parse__binary_file_with_missing_paths(void)
strlen(PATCH_BINARY_FILE_WITH_MISSING_PATHS), NULL));
}
+void test_patch_parse__binary_file_with_whitespace_paths(void)
+{
+ git_patch *patch;
+ cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_WITH_WHITESPACE_PATHS,
+ strlen(PATCH_BINARY_FILE_WITH_WHITESPACE_PATHS), NULL));
+}
+
+void test_patch_parse__binary_file_with_empty_quoted_paths(void)
+{
+ git_patch *patch;
+ cl_git_fail(git_patch_from_buffer(&patch, PATCH_BINARY_FILE_WITH_QUOTED_EMPTY_PATHS,
+ strlen(PATCH_BINARY_FILE_WITH_QUOTED_EMPTY_PATHS), NULL));
+}
+
void test_patch_parse__memory_leak_on_multiple_paths(void)
{
git_patch *patch;
diff --git a/tests/patch/patch_common.h b/tests/patch/patch_common.h
index 153bab5..4f2141d 100644
--- a/tests/patch/patch_common.h
+++ b/tests/patch/patch_common.h
@@ -912,6 +912,18 @@
"+++ \n" \
"Binary files "
+#define PATCH_BINARY_FILE_WITH_WHITESPACE_PATHS \
+ "diff --git a/file b/file\n" \
+ "--- \n" \
+ "+++ \n" \
+ "Binary files "
+
+#define PATCH_BINARY_FILE_WITH_QUOTED_EMPTY_PATHS \
+ "diff --git a/file b/file\n" \
+ "--- \"\"\n" \
+ "+++ \"\"\n" \
+ "Binary files "
+
#define PATCH_MULTIPLE_OLD_PATHS \
"diff --git \n" \
"--- \n" \