Commit 223e7e43efffdcab4da864413e70eff40e8ada46
2019-10-19T15:42:54
patch_parse: reject patches with multiple old/new paths It's currently possible to have patches with multiple old path name headers. As we didn't check for this case, this resulted in a memory leak when overwriting the old old path with the new old path because we simply discarded the old pointer. Instead of fixing this by free'ing the old pointer, we should reject such patches altogether. It doesn't make any sense for the "---" or "+++" markers to occur multiple times within a patch n the first place. This also implicitly fixes the memory leak.
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
diff --git a/src/patch_parse.c b/src/patch_parse.c
index 1269182..a718432 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -91,10 +91,14 @@ done:
static int parse_header_path(char **out, git_patch_parse_ctx *ctx)
{
git_buf path = GIT_BUF_INIT;
- int error = parse_header_path_buf(&path, ctx, header_path_len(ctx));
+ int error;
+ if ((error = parse_header_path_buf(&path, ctx, header_path_len(ctx))) < 0)
+ goto out;
*out = git_buf_detach(&path);
+out:
+ git_buf_dispose(&path);
return error;
}
@@ -104,6 +108,12 @@ static int parse_header_git_oldpath(
git_buf old_path = GIT_BUF_INIT;
int error;
+ if (patch->old_path) {
+ error = git_parse_err("patch contains duplicate old path at line %"PRIuZ,
+ ctx->parse_ctx.line_num);
+ goto out;
+ }
+
if ((error = parse_header_path_buf(&old_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
goto out;
@@ -120,9 +130,14 @@ static int parse_header_git_newpath(
git_buf new_path = GIT_BUF_INIT;
int error;
- if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
+ if (patch->new_path) {
+ error = git_parse_err("patch contains duplicate new path at line %"PRIuZ,
+ ctx->parse_ctx.line_num);
goto out;
+ }
+ if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
+ goto out;
patch->new_path = git_buf_detach(&new_path);
out:
diff --git a/tests/patch/parse.c b/tests/patch/parse.c
index b89322f..fb185eb 100644
--- a/tests/patch/parse.c
+++ b/tests/patch/parse.c
@@ -148,3 +148,9 @@ void test_patch_parse__lifetime_of_patch_does_not_depend_on_buffer(void)
git_patch_free(patch);
}
+
+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));
+}
diff --git a/tests/patch/patch_common.h b/tests/patch/patch_common.h
index d730d14..68b183d 100644
--- a/tests/patch/patch_common.h
+++ b/tests/patch/patch_common.h
@@ -905,3 +905,10 @@
"-b\n" \
"+bb\n" \
" c\n"
+
+#define PATCH_MULTIPLE_OLD_PATHS \
+ "diff --git \n" \
+ "--- \n" \
+ "+++ \n" \
+ "index 0000..7DDb\n" \
+ "--- \n"