Skip to content

Commit 223e7e4

Browse files
committed
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 parent b246bed commit 223e7e4

File tree

3 files changed

+30
-2
lines changed

3 files changed

+30
-2
lines changed

src/patch_parse.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,14 @@ static int parse_header_path_buf(git_buf *path, git_patch_parse_ctx *ctx, size_t
9191
static int parse_header_path(char **out, git_patch_parse_ctx *ctx)
9292
{
9393
git_buf path = GIT_BUF_INIT;
94-
int error = parse_header_path_buf(&path, ctx, header_path_len(ctx));
94+
int error;
9595

96+
if ((error = parse_header_path_buf(&path, ctx, header_path_len(ctx))) < 0)
97+
goto out;
9698
*out = git_buf_detach(&path);
9799

100+
out:
101+
git_buf_dispose(&path);
98102
return error;
99103
}
100104

@@ -104,6 +108,12 @@ static int parse_header_git_oldpath(
104108
git_buf old_path = GIT_BUF_INIT;
105109
int error;
106110

111+
if (patch->old_path) {
112+
error = git_parse_err("patch contains duplicate old path at line %"PRIuZ,
113+
ctx->parse_ctx.line_num);
114+
goto out;
115+
}
116+
107117
if ((error = parse_header_path_buf(&old_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
108118
goto out;
109119

@@ -120,9 +130,14 @@ static int parse_header_git_newpath(
120130
git_buf new_path = GIT_BUF_INIT;
121131
int error;
122132

123-
if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
133+
if (patch->new_path) {
134+
error = git_parse_err("patch contains duplicate new path at line %"PRIuZ,
135+
ctx->parse_ctx.line_num);
124136
goto out;
137+
}
125138

139+
if ((error = parse_header_path_buf(&new_path, ctx, ctx->parse_ctx.line_len - 1)) < 0)
140+
goto out;
126141
patch->new_path = git_buf_detach(&new_path);
127142

128143
out:

tests/patch/parse.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,9 @@ void test_patch_parse__lifetime_of_patch_does_not_depend_on_buffer(void)
148148

149149
git_patch_free(patch);
150150
}
151+
152+
void test_patch_parse__memory_leak_on_multiple_paths(void)
153+
{
154+
git_patch *patch;
155+
cl_git_fail(git_patch_from_buffer(&patch, PATCH_MULTIPLE_OLD_PATHS, strlen(PATCH_MULTIPLE_OLD_PATHS), NULL));
156+
}

tests/patch/patch_common.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,3 +905,10 @@
905905
"-b\n" \
906906
"+bb\n" \
907907
" c\n"
908+
909+
#define PATCH_MULTIPLE_OLD_PATHS \
910+
"diff --git \n" \
911+
"--- \n" \
912+
"+++ \n" \
913+
"index 0000..7DDb\n" \
914+
"--- \n"

0 commit comments

Comments
 (0)