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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Jun 19, 2014 at 17:30 UTC
45bc131dd3e1eb6edd903957cf9d42f37ad02181
1 file changed
+26
-15
merge-recursive.c
+26
-15
@@ -601,25 +601,36 @@ static int remove_file(struct merge_options *o, int clean,
601
return 0;
602
}
603
604
+/* add a string to a strbuf, but converting "/" to "_" */
605
+static void add_flattened_path(struct strbuf *out, const char *s)
606
+{
607
+ size_t i = out->len;
608
+ strbuf_addstr(out, s);
609
+ for (; i < out->len; i++)
610
+ if (out->buf[i] == '/')
611
+ out->buf[i] = '_';
612
+}
613
+
614
static char *unique_path(struct merge_options *o, const char *path, const char *branch)
615
{
606
- char *newpath = xmalloc(strlen(path) + 1 + strlen(branch) + 8 + 1);
616
+ struct strbuf newpath = STRBUF_INIT;
617
int suffix = 0;
618
struct stat st;
609
- char *p = newpath + strlen(path);
610
- strcpy(newpath, path);
611
- *(p++) = '~';
612
- strcpy(p, branch);
613
- for (; *p; ++p)
614
- if ('/' == *p)
615
- *p = '_';
616
- while (string_list_has_string(&o->current_file_set, newpath) ||
617
- string_list_has_string(&o->current_directory_set, newpath) ||
618
- lstat(newpath, &st) == 0)
619
- sprintf(p, "_%d", suffix++);
620
-
621
- string_list_insert(&o->current_file_set, newpath);
622
- return newpath;
619
+ size_t base_len;
620
+
621
+ strbuf_addf(&newpath, "%s~", path);
622
+ add_flattened_path(&newpath, branch);
623
+
624
+ base_len = newpath.len;
625
+ while (string_list_has_string(&o->current_file_set, newpath.buf) ||
626
+ string_list_has_string(&o->current_directory_set, newpath.buf) ||
627
+ lstat(newpath.buf, &st) == 0) {
628
+ strbuf_setlen(&newpath, base_len);
629
+ strbuf_addf(&newpath, "_%d", suffix++);
630
+ }
631
+
632
+ string_list_insert(&o->current_file_set, newpath.buf);
633
+ return strbuf_detach(&newpath, NULL);
634
}
635
636
static int dir_in_way(const char *path, int check_working_copy)