use st_add and st_mult for allocation size computation

If our size computation overflows size_t, we may allocate a much smaller buffer than we expected and overflow it. It's probably impossible to trigger an overflow in most of these sites in practice, but it is easy enough convert their additions and multiplications into overflow-checking variants. This may be fixing real bugs, and it makes auditing the code easier. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Feb 22, 2016 at 17:44 UTC 50a6c8efa2bbeddf46ca34c7765024108202e04b
25 files changed +56 -53
archive.c
+2 -2
@@ -171,8 +171,8 @@ static void queue_directory(const unsigned char *sha1,
171 unsigned mode, int stage, struct archiver_context *c)
172 {
173 struct directory *d;
174 - size_t len = base->len + 1 + strlen(filename) + 1;
175 - d = xmalloc(sizeof(*d) + len);
174 + size_t len = st_add4(base->len, 1, strlen(filename), 1);
175 + d = xmalloc(st_add(sizeof(*d), len));
176 d->up = c->bottom;
177 d->baselen = base->len;
178 d->mode = mode;
builtin/apply.c
+1 -1
@@ -2632,7 +2632,7 @@ static void update_image(struct image *img,
2632 insert_count = postimage->len;
2633
2634 /* Adjust the contents */
2635 - result = xmalloc(img->len + insert_count - remove_count + 1);
2635 + result = xmalloc(st_add3(st_sub(img->len, remove_count), insert_count, 1));
2636 memcpy(result, img->buf, applied_at);
2637 memcpy(result + applied_at, postimage->buf, postimage->len);
2638 memcpy(result + applied_at + postimage->len,
builtin/clean.c
+1 -1
@@ -615,7 +615,7 @@ static int *list_and_choose(struct menu_opts *opts, struct menu_stuff *stuff)
615 nr += chosen[i];
616 }
617
618 - result = xcalloc(nr + 1, sizeof(int));
618 + result = xcalloc(st_add(nr, 1), sizeof(int));
619 for (i = 0; i < stuff->nr && j < nr; i++) {
620 if (chosen[i])
621 result[j++] = i;
builtin/fetch.c
+1 -1
@@ -1107,7 +1107,7 @@ static int fetch_one(struct remote *remote, int argc, const char **argv)
1107 if (argc > 0) {
1108 int j = 0;
1109 int i;
1110 - refs = xcalloc(argc + 1, sizeof(const char *));
1110 + refs = xcalloc(st_add(argc, 1), sizeof(const char *));
1111 for (i = 0; i < argc; i++) {
1112 if (!strcmp(argv[i], "tag")) {
1113 i++;
builtin/index-pack.c
+2 -2
@@ -1744,9 +1744,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
1744
1745 curr_pack = open_pack_file(pack_name);
1746 parse_pack_header();
1747 - objects = xcalloc(nr_objects + 1, sizeof(struct object_entry));
1747 + objects = xcalloc(st_add(nr_objects, 1), sizeof(struct object_entry));
1748 if (show_stat)
1749 - obj_stat = xcalloc(nr_objects + 1, sizeof(struct object_stat));
1749 + obj_stat = xcalloc(st_add(nr_objects, 1), sizeof(struct object_stat));
1750 ofs_deltas = xcalloc(nr_objects, sizeof(struct ofs_delta_entry));
1751 parse_pack_objects(pack_sha1);
1752 resolve_deltas();
builtin/merge.c
+1 -1
@@ -939,7 +939,7 @@ static int setup_with_upstream(const char ***argv)
939 if (!branch->merge_nr)
940 die(_("No default upstream defined for the current branch."));
941
942 - args = xcalloc(branch->merge_nr + 1, sizeof(char *));
942 + args = xcalloc(st_add(branch->merge_nr, 1), sizeof(char *));
943 for (i = 0; i < branch->merge_nr; i++) {
944 if (!branch->merge[i]->dst)
945 die(_("No remote-tracking branch for %s from %s"),
builtin/mv.c
+2 -2
@@ -48,9 +48,9 @@ static const char **internal_copy_pathspec(const char *prefix,
48
49 static const char *add_slash(const char *path)
50 {
51 - int len = strlen(path);
51 + size_t len = strlen(path);
52 if (path[len - 1] != '/') {
53 - char *with_slash = xmalloc(len + 2);
53 + char *with_slash = xmalloc(st_add(len, 2));
54 memcpy(with_slash, path, len);
55 with_slash[len++] = '/';
56 with_slash[len] = 0;
builtin/receive-pack.c
+1 -1
@@ -1372,7 +1372,7 @@ static struct command **queue_command(struct command **tail,
1372
1373 refname = line + 82;
1374 reflen = linelen - 82;
1375 - cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
1375 + cmd = xcalloc(1, st_add3(sizeof(struct command), reflen, 1));
1376 hashcpy(cmd->old_sha1, old_sha1);
1377 hashcpy(cmd->new_sha1, new_sha1);
1378 memcpy(cmd->ref_name, refname, reflen);
combine-diff.c
+7 -7
@@ -189,11 +189,11 @@ static struct lline *coalesce_lines(struct lline *base, int *lenbase,
189 * - Else if we have NEW, insert newend lline into base and
190 * consume newend
191 */
192 - lcs = xcalloc(origbaselen + 1, sizeof(int*));
193 - directions = xcalloc(origbaselen + 1, sizeof(enum coalesce_direction*));
192 + lcs = xcalloc(st_add(origbaselen, 1), sizeof(int*));
193 + directions = xcalloc(st_add(origbaselen, 1), sizeof(enum coalesce_direction*));
194 for (i = 0; i < origbaselen + 1; i++) {
195 - lcs[i] = xcalloc(lennew + 1, sizeof(int));
196 - directions[i] = xcalloc(lennew + 1, sizeof(enum coalesce_direction));
195 + lcs[i] = xcalloc(st_add(lennew, 1), sizeof(int));
196 + directions[i] = xcalloc(st_add(lennew, 1), sizeof(enum coalesce_direction));
197 directions[i][0] = BASE;
198 }
199 for (j = 1; j < lennew + 1; j++)
@@ -1111,7 +1111,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
1111 if (result_size && result[result_size-1] != '\n')
1112 cnt++; /* incomplete line */
1113
1114 - sline = xcalloc(cnt+2, sizeof(*sline));
1114 + sline = xcalloc(st_add(cnt, 2), sizeof(*sline));
1115 sline[0].bol = result;
1116 for (lno = 0, cp = result; cp < result + result_size; cp++) {
1117 if (*cp == '\n') {
@@ -1130,7 +1130,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
1130 /* Even p_lno[cnt+1] is valid -- that is for the end line number
1131 * for deletion hunk at the end.
1132 */
1133 - sline[0].p_lno = xcalloc((cnt+2) * num_parent, sizeof(unsigned long));
1133 + sline[0].p_lno = xcalloc(st_mult(st_add(cnt, 2), num_parent), sizeof(unsigned long));
1134 for (lno = 0; lno <= cnt; lno++)
1135 sline[lno+1].p_lno = sline[lno].p_lno + num_parent;
1136
@@ -1262,7 +1262,7 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,
1262 struct diff_filespec *pool;
1263
1264 pair = xmalloc(sizeof(*pair));
1265 - pool = xcalloc(num_parent + 1, sizeof(struct diff_filespec));
1265 + pool = xcalloc(st_add(num_parent, 1), sizeof(struct diff_filespec));
1266 pair->one = pool + 1;
1267 pair->two = pool;
1268
commit.c
+1 -1
@@ -147,7 +147,7 @@ struct commit_graft *read_graft_line(char *buf, int len)
147 if ((len + 1) % entry_size)
148 goto bad_graft_data;
149 i = (len + 1) / entry_size - 1;
150 - graft = xmalloc(sizeof(*graft) + GIT_SHA1_RAWSZ * i);
150 + graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
151 graft->nr_parent = i;
152 if (get_oid_hex(buf, &graft->oid))
153 goto bad_graft_data;
compat/mingw.c
+2 -2
@@ -769,7 +769,7 @@ static const char *quote_arg(const char *arg)
769 return arg;
770
771 /* insert \ where necessary */
772 - d = q = xmalloc(len+n+3);
772 + d = q = xmalloc(st_add3(len, n, 3));
773 *d++ = '"';
774 while (*arg) {
775 if (*arg == '"')
@@ -1028,7 +1028,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
1028 free(quoted);
1029 }
1030
1031 - wargs = xmalloc((2 * args.len + 1) * sizeof(wchar_t));
1031 + wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
1032 xutftowcs(wargs, args.buf, 2 * args.len + 1);
1033 strbuf_release(&args);
1034
compat/qsort.c
+1 -1
@@ -47,7 +47,7 @@ static void msort_with_tmp(void *b, size_t n, size_t s,
47 void git_qsort(void *b, size_t n, size_t s,
48 int (*cmp)(const void *, const void *))
49 {
50 - const size_t size = n * s;
50 + const size_t size = st_mult(n, s);
51 char buf[1024];
52
53 if (size < sizeof(buf)) {
compat/setenv.c
+1 -1
@@ -18,7 +18,7 @@ int gitsetenv(const char *name, const char *value, int replace)
18
19 namelen = strlen(name);
20 valuelen = strlen(value);
21 - envstr = malloc((namelen + valuelen + 2));
21 + envstr = malloc(st_add3(namelen, valuelen, 2));
22 if (!envstr) {
23 errno = ENOMEM;
24 return -1;
compat/win32/syslog.c
+2 -2
@@ -32,7 +32,7 @@ void syslog(int priority, const char *fmt, ...)
32 return;
33 }
34
35 - str = malloc(str_len + 1);
35 + str = malloc(st_add(str_len, 1));
36 if (!str) {
37 warning("malloc failed: '%s'", strerror(errno));
38 return;
@@ -43,7 +43,7 @@ void syslog(int priority, const char *fmt, ...)
43 va_end(ap);
44
45 while ((pos = strstr(str, "%1")) != NULL) {
46 - str = realloc(str, ++str_len + 1);
46 + str = realloc(str, st_add(++str_len, 1));
47 if (!str) {
48 warning("realloc failed: '%s'", strerror(errno));
49 return;
diffcore-delta.c
+4 -2
@@ -53,7 +53,8 @@ static struct spanhash_top *spanhash_rehash(struct spanhash_top *orig)
53 int osz = 1 << orig->alloc_log2;
54 int sz = osz << 1;
55
56 - new = xmalloc(sizeof(*orig) + sizeof(struct spanhash) * sz);
56 + new = xmalloc(st_add(sizeof(*orig),
57 + st_mult(sizeof(struct spanhash), sz)));
58 new->alloc_log2 = orig->alloc_log2 + 1;
59 new->free = INITIAL_FREE(new->alloc_log2);
60 memset(new->data, 0, sizeof(struct spanhash) * sz);
@@ -130,7 +131,8 @@ static struct spanhash_top *hash_chars(struct diff_filespec *one)
131 int is_text = !diff_filespec_is_binary(one);
132
133 i = INITIAL_HASH_SIZE;
133 - hash = xmalloc(sizeof(*hash) + sizeof(struct spanhash) * (1<<i));
134 + hash = xmalloc(st_add(sizeof(*hash),
135 + st_mult(sizeof(struct spanhash), 1<<i)));
136 hash->alloc_log2 = i;
137 hash->free = INITIAL_FREE(i);
138 memset(hash->data, 0, sizeof(struct spanhash) * (1<<i));
diffcore-rename.c
+1 -1
@@ -537,7 +537,7 @@ void diffcore_rename(struct diff_options *options)
537 rename_dst_nr * rename_src_nr, 50, 1);
538 }
539
540 - mx = xcalloc(num_create * NUM_CANDIDATE_PER_DST, sizeof(*mx));
540 + mx = xcalloc(st_mult(num_create, NUM_CANDIDATE_PER_DST), sizeof(*mx));
541 for (dst_cnt = i = 0; i < rename_dst_nr; i++) {
542 struct diff_filespec *two = rename_dst[i].two;
543 struct diff_score *m;
dir.c
+2 -2
@@ -689,7 +689,7 @@ static int add_excludes(const char *fname, const char *base, int baselen,
689 return 0;
690 }
691 if (buf[size-1] != '\n') {
692 - buf = xrealloc(buf, size+1);
692 + buf = xrealloc(buf, st_add(size, 1));
693 buf[size++] = '\n';
694 }
695 } else {
@@ -2452,7 +2452,7 @@ static int read_one_dir(struct untracked_cache_dir **untracked_,
2452 next = data + len + 1;
2453 if (next > rd->end)
2454 return -1;
2455 - *untracked_ = untracked = xmalloc(sizeof(*untracked) + len);
2455 + *untracked_ = untracked = xmalloc(st_add(sizeof(*untracked), len));
2456 memcpy(untracked, &ud, sizeof(ud));
2457 memcpy(untracked->name, data, len + 1);
2458 data = next;
fast-import.c
+1 -1
@@ -622,7 +622,7 @@ static void *pool_alloc(size_t len)
622 return xmalloc(len);
623 }
624 total_allocd += sizeof(struct mem_pool) + mem_pool_alloc;
625 - p = xmalloc(sizeof(struct mem_pool) + mem_pool_alloc);
625 + p = xmalloc(st_add(sizeof(struct mem_pool), mem_pool_alloc));
626 p->next_pool = mem_pool;
627 p->next_free = (char *) p->space;
628 p->end = p->next_free + mem_pool_alloc;
refs.c
+1 -1
@@ -906,7 +906,7 @@ char *shorten_unambiguous_ref(const char *refname, int strict)
906 /* -2 for strlen("%.*s") - strlen("%s"); +1 for NUL */
907 total_len += strlen(ref_rev_parse_rules[nr_rules]) - 2 + 1;
908
909 - scanf_fmts = xmalloc(nr_rules * sizeof(char *) + total_len);
909 + scanf_fmts = xmalloc(st_add(st_mult(nr_rules, sizeof(char *)), total_len));
910
911 offset = 0;
912 for (i = 0; i < nr_rules; i++) {
remote.c
+4 -4
@@ -928,7 +928,7 @@ static struct ref *alloc_ref_with_prefix(const char *prefix, size_t prefixlen,
928 const char *name)
929 {
930 size_t len = strlen(name);
931 - struct ref *ref = xcalloc(1, sizeof(struct ref) + prefixlen + len + 1);
931 + struct ref *ref = xcalloc(1, st_add4(sizeof(*ref), prefixlen, len, 1));
932 memcpy(ref->name, prefix, prefixlen);
933 memcpy(ref->name + prefixlen, name, len);
934 return ref;
@@ -945,9 +945,9 @@ struct ref *copy_ref(const struct ref *ref)
945 size_t len;
946 if (!ref)
947 return NULL;
948 - len = strlen(ref->name);
949 - cpy = xmalloc(sizeof(struct ref) + len + 1);
950 - memcpy(cpy, ref, sizeof(struct ref) + len + 1);
948 + len = st_add3(sizeof(struct ref), strlen(ref->name), 1);
949 + cpy = xmalloc(len);
950 + memcpy(cpy, ref, len);
951 cpy->next = NULL;
952 cpy->symref = xstrdup_or_null(ref->symref);
953 cpy->remote_status = xstrdup_or_null(ref->remote_status);
revision.c
+1 -1
@@ -540,7 +540,7 @@ struct treesame_state {
540 static struct treesame_state *initialise_treesame(struct rev_info *revs, struct commit *commit)
541 {
542 unsigned n = commit_list_count(commit->parents);
543 - struct treesame_state *st = xcalloc(1, sizeof(*st) + n);
543 + struct treesame_state *st = xcalloc(1, st_add(sizeof(*st), n));
544 st->nparents = n;
545 add_decoration(&revs->treesame, &commit->object, st);
546 return st;
sha1_file.c
+11 -9
@@ -253,7 +253,7 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
253 {
254 struct alternate_object_database *ent;
255 struct alternate_object_database *alt;
256 - int pfxlen, entlen;
256 + size_t pfxlen, entlen;
257 struct strbuf pathbuf = STRBUF_INIT;
258
259 if (!is_absolute_path(entry) && relative_base) {
@@ -273,8 +273,8 @@ static int link_alt_odb_entry(const char *entry, const char *relative_base,
273 while (pfxlen && pathbuf.buf[pfxlen-1] == '/')
274 pfxlen -= 1;
275
276 - entlen = pfxlen + 43; /* '/' + 2 hex + '/' + 38 hex + NUL */
277 - ent = xmalloc(sizeof(*ent) + entlen);
276 + entlen = st_add(pfxlen, 43); /* '/' + 2 hex + '/' + 38 hex + NUL */
277 + ent = xmalloc(st_add(sizeof(*ent), entlen));
278 memcpy(ent->base, pathbuf.buf, pfxlen);
279 strbuf_release(&pathbuf);
280
@@ -1134,7 +1134,7 @@ unsigned char *use_pack(struct packed_git *p,
1134
1135 static struct packed_git *alloc_packed_git(int extra)
1136 {
1137 - struct packed_git *p = xmalloc(sizeof(*p) + extra);
1137 + struct packed_git *p = xmalloc(st_add(sizeof(*p), extra));
1138 memset(p, 0, sizeof(*p));
1139 p->pack_fd = -1;
1140 return p;
@@ -1168,7 +1168,7 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
1168 * ".pack" is long enough to hold any suffix we're adding (and
1169 * the use xsnprintf double-checks that)
1170 */
1171 - alloc = path_len + strlen(".pack") + 1;
1171 + alloc = st_add3(path_len, strlen(".pack"), 1);
1172 p = alloc_packed_git(alloc);
1173 memcpy(p->pack_name, path, path_len);
1174
@@ -1196,7 +1196,7 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)
1196 struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path)
1197 {
1198 const char *path = sha1_pack_name(sha1);
1199 - int alloc = strlen(path) + 1;
1199 + size_t alloc = st_add(strlen(path), 1);
1200 struct packed_git *p = alloc_packed_git(alloc);
1201
1202 memcpy(p->pack_name, path, alloc); /* includes NUL */
@@ -1413,10 +1413,12 @@ static void mark_bad_packed_object(struct packed_git *p,
1413 {
1414 unsigned i;
1415 for (i = 0; i < p->num_bad_objects; i++)
1416 - if (!hashcmp(sha1, p->bad_object_sha1 + 20 * i))
1416 + if (!hashcmp(sha1, p->bad_object_sha1 + GIT_SHA1_RAWSZ * i))
1417 return;
1418 - p->bad_object_sha1 = xrealloc(p->bad_object_sha1, 20 * (p->num_bad_objects + 1));
1419 - hashcpy(p->bad_object_sha1 + 20 * p->num_bad_objects, sha1);
1418 + p->bad_object_sha1 = xrealloc(p->bad_object_sha1,
1419 + st_mult(GIT_SHA1_RAWSZ,
1420 + st_add(p->num_bad_objects, 1)));
1421 + hashcpy(p->bad_object_sha1 + GIT_SHA1_RAWSZ * p->num_bad_objects, sha1);
1422 p->num_bad_objects++;
1423 }
1424
sha1_name.c
+2 -3
@@ -87,9 +87,8 @@ static void find_short_object_filename(int len, const char *hex_pfx, struct disa
87 * object databases including our own.
88 */
89 const char *objdir = get_object_directory();
90 - int objdir_len = strlen(objdir);
91 - int entlen = objdir_len + 43;
92 - fakeent = xmalloc(sizeof(*fakeent) + entlen);
90 + size_t objdir_len = strlen(objdir);
91 + fakeent = xmalloc(st_add3(sizeof(*fakeent), objdir_len, 43));
92 memcpy(fakeent->base, objdir, objdir_len);
93 fakeent->name = fakeent->base + objdir_len + 1;
94 fakeent->name[-1] = '/';
shallow.c
+1 -1
@@ -389,7 +389,7 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,
389 unsigned int i, nr;
390 struct commit_list *head = NULL;
391 int bitmap_nr = (info->nr_bits + 31) / 32;
392 - int bitmap_size = bitmap_nr * sizeof(uint32_t);
392 + size_t bitmap_size = st_mult(bitmap_nr, sizeof(uint32_t));
393 uint32_t *tmp = xmalloc(bitmap_size); /* to be freed before return */
394 uint32_t *bitmap = paint_alloc(info);
395 struct commit *c = lookup_commit_reference_gently(sha1, 1);
submodule.c
+3 -3
@@ -122,7 +122,7 @@ static int add_submodule_odb(const char *path)
122 struct strbuf objects_directory = STRBUF_INIT;
123 struct alternate_object_database *alt_odb;
124 int ret = 0;
125 - int alloc;
125 + size_t alloc;
126
127 strbuf_git_path_submodule(&objects_directory, path, "objects/");
128 if (!is_directory(objects_directory.buf)) {
@@ -137,8 +137,8 @@ static int add_submodule_odb(const char *path)
137 objects_directory.len))
138 goto done;
139
140 - alloc = objects_directory.len + 42; /* for "12/345..." sha1 */
141 - alt_odb = xmalloc(sizeof(*alt_odb) + alloc);
140 + alloc = st_add(objects_directory.len, 42); /* for "12/345..." sha1 */
141 + alt_odb = xmalloc(st_add(sizeof(*alt_odb), alloc));
142 alt_odb->next = alt_odb_list;
143 xsnprintf(alt_odb->base, alloc, "%s", objects_directory.buf);
144 alt_odb->name = alt_odb->base + objects_directory.len;