Skip to content

Commit 45bc131

Browse files
peffgitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent f332069 commit 45bc131

File tree

1 file changed

+26
-15
lines changed

1 file changed

+26
-15
lines changed

merge-recursive.c

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -601,25 +601,36 @@ static int remove_file(struct merge_options *o, int clean,
601601
return 0;
602602
}
603603

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+
604614
static char *unique_path(struct merge_options *o, const char *path, const char *branch)
605615
{
606-
char *newpath = xmalloc(strlen(path) + 1 + strlen(branch) + 8 + 1);
616+
struct strbuf newpath = STRBUF_INIT;
607617
int suffix = 0;
608618
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);
623634
}
624635

625636
static int dir_in_way(const char *path, int check_working_copy)

0 commit comments

Comments
 (0)