From 7ce231ff3491357e4fe2e76d356aee8fed57cfe2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt <ps@pks.im> Date: Tue, 5 Nov 2019 22:44:27 +0100 Subject: [PATCH] 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. --- src/patch_parse.c | 21 +++++++++------------ tests/patch/parse.c | 14 ++++++++++++++ tests/patch/patch_common.h | 12 ++++++++++++ 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/src/patch_parse.c b/src/patch_parse.c index 87aab96..2f18b87 100644 --- a/src/patch_parse.c +++ b/src/patch_parse.c @@ -57,27 +57,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" \ -- libgit2 0.26.0