From 283101869bea8feb5d58f6ea1b568e9b197526d3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 19 Jun 2014 17:24:33 -0400 Subject: use xstrfmt to replace xmalloc + sprintf This is one line shorter, and makes sure the length in the malloc and sprintf steps match. These conversions are very straightforward; we can drop the malloc entirely, and replace the sprintf with xstrfmt. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- merge-recursive.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index cab16fafb..532a1da2b 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -969,14 +969,10 @@ merge_file_special_markers(struct merge_options *o, char *side2 = NULL; struct merge_file_info mfi; - if (filename1) { - side1 = xmalloc(strlen(branch1) + strlen(filename1) + 2); - sprintf(side1, "%s:%s", branch1, filename1); - } - if (filename2) { - side2 = xmalloc(strlen(branch2) + strlen(filename2) + 2); - sprintf(side2, "%s:%s", branch2, filename2); - } + if (filename1) + side1 = xstrfmt("%s:%s", branch1, filename1); + if (filename2) + side2 = xstrfmt("%s:%s", branch2, filename2); mfi = merge_file_1(o, one, a, b, side1 ? side1 : branch1, side2 ? side2 : branch2); -- cgit v1.2.1 From 45bc131dd3e1eb6edd903957cf9d42f37ad02181 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 19 Jun 2014 17:30:26 -0400 Subject: unique_path: fix unlikely heap overflow When merge-recursive creates a unique filename, it uses a template like: path~branch_%d where the final "_%d" is filled by an incrementing counter until we find a unique name. We allocate 8 characters for the counter, but there is no logic to limit the size of the integer. Of course, this is extremely unlikely, as you would need a hundred million collisions to trigger the problem. Even if an attacker constructed a specialized repo, it is unlikely that the victim would have the patience to run the merge. However, we can make it trivially correct (and hopefully more readable) by using a strbuf. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- merge-recursive.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 532a1da2b..398a734f3 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -601,25 +601,36 @@ static int remove_file(struct merge_options *o, int clean, return 0; } +/* add a string to a strbuf, but converting "/" to "_" */ +static void add_flattened_path(struct strbuf *out, const char *s) +{ + size_t i = out->len; + strbuf_addstr(out, s); + for (; i < out->len; i++) + if (out->buf[i] == '/') + out->buf[i] = '_'; +} + static char *unique_path(struct merge_options *o, const char *path, const char *branch) { - char *newpath = xmalloc(strlen(path) + 1 + strlen(branch) + 8 + 1); + struct strbuf newpath = STRBUF_INIT; int suffix = 0; struct stat st; - char *p = newpath + strlen(path); - strcpy(newpath, path); - *(p++) = '~'; - strcpy(p, branch); - for (; *p; ++p) - if ('/' == *p) - *p = '_'; - while (string_list_has_string(&o->current_file_set, newpath) || - string_list_has_string(&o->current_directory_set, newpath) || - lstat(newpath, &st) == 0) - sprintf(p, "_%d", suffix++); - - string_list_insert(&o->current_file_set, newpath); - return newpath; + size_t base_len; + + strbuf_addf(&newpath, "%s~", path); + add_flattened_path(&newpath, branch); + + base_len = newpath.len; + while (string_list_has_string(&o->current_file_set, newpath.buf) || + string_list_has_string(&o->current_directory_set, newpath.buf) || + lstat(newpath.buf, &st) == 0) { + strbuf_setlen(&newpath, base_len); + strbuf_addf(&newpath, "_%d", suffix++); + } + + string_list_insert(&o->current_file_set, newpath.buf); + return strbuf_detach(&newpath, NULL); } static int dir_in_way(const char *path, int check_working_copy) -- cgit v1.2.1