From 392702ee2c88d7d8aaff25f7a84acb73606f9094 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 9 Feb 2015 23:41:13 -0500 Subject: [PATCH 01/15] allocations: test for overflow of requested size Introduce some helper macros to test integer overflow from arithmetic and set error message appropriately. --- src/array.h | 27 ++++++++++---- src/blame.c | 28 +++++++++++++-- src/blame_git.c | 8 +++-- src/buf_text.c | 12 ++++++- src/buffer.c | 62 +++++++++++++++++++++++++++++--- src/common.h | 22 ++++++++++++ src/config_file.c | 33 ++++++++++++----- src/delta-apply.c | 1 + src/delta.c | 38 ++++++++++++++++---- src/diff.c | 2 ++ src/diff_driver.c | 42 ++++++++++++++++------ src/diff_patch.c | 12 ++++++- src/diff_tform.c | 1 + src/filebuf.c | 5 +-- src/fileops.c | 6 +++- src/filter.c | 17 +++++++-- src/index.c | 18 +++++++--- src/iterator.c | 11 ++++-- src/merge.c | 2 ++ src/notes.c | 2 +- src/odb.c | 1 + src/odb_loose.c | 12 ++++++- src/odb_mempack.c | 1 + src/oid.c | 1 + src/pack-objects.c | 22 ++++++++++-- src/pack.c | 7 ++++ src/path.c | 64 +++++++++++++++++++++++++-------- src/pool.c | 6 ++-- src/refs.c | 18 +++++++--- src/remote.c | 3 +- src/revwalk.c | 6 +--- src/sortedcache.c | 4 ++- src/tag.c | 2 ++ src/transports/cred.c | 3 ++ src/transports/git.c | 4 +-- src/transports/local.c | 2 +- src/transports/smart.c | 2 +- src/transports/smart_pkt.c | 31 ++++++++++++++-- src/transports/smart_protocol.c | 4 +-- src/transports/winhttp.c | 4 +-- src/tree.c | 18 +++++++--- src/tsort.c | 5 ++- src/util.h | 15 ++++++-- src/vector.c | 7 ++-- src/win32/dir.c | 8 +++-- src/win32/utf-conv.c | 5 ++- src/zstream.c | 1 + tests/core/errors.c | 40 +++++++++++++++++++++ 48 files changed, 528 insertions(+), 117 deletions(-) diff --git a/src/array.h b/src/array.h index af9eafa43..ca5a35ffe 100644 --- a/src/array.h +++ b/src/array.h @@ -45,15 +45,28 @@ typedef git_array_t(char) git_array_generic_t; GIT_INLINE(void *) git_array_grow(void *_a, size_t item_size) { volatile git_array_generic_t *a = _a; - uint32_t new_size = (a->size < 8) ? 8 : a->asize * 3 / 2; - char *new_array = git__realloc(a->ptr, new_size * item_size); - if (!new_array) { - git_array_clear(*a); - return NULL; + uint32_t new_size; + char *new_array; + + if (a->size < 8) { + new_size = 8; } else { - a->ptr = new_array; a->asize = new_size; a->size++; - return a->ptr + (a->size - 1) * item_size; + if (GIT_ALLOC_OVERFLOW_MULTIPLY(a->size, 3 / 2)) + goto on_oom; + + new_size = a->size * 3 / 2; } + + if (GIT_ALLOC_OVERFLOW_MULTIPLY(new_size, item_size) || + (new_array = git__realloc(a->ptr, new_size * item_size)) == NULL) + goto on_oom; + + a->ptr = new_array; a->asize = new_size; a->size++; + return a->ptr + (a->size - 1) * item_size; + +on_oom: + git_array_clear(*a); + return NULL; } #define git_array_alloc(a) \ diff --git a/src/blame.c b/src/blame.c index 2cc5e552b..4a12cb85b 100644 --- a/src/blame.c +++ b/src/blame.c @@ -76,6 +76,10 @@ static git_blame_hunk* dup_hunk(git_blame_hunk *hunk) hunk->lines_in_hunk, hunk->orig_start_line_number, hunk->orig_path); + + if (!newhunk) + return NULL; + git_oid_cpy(&newhunk->orig_commit_id, &hunk->orig_commit_id); git_oid_cpy(&newhunk->final_commit_id, &hunk->final_commit_id); newhunk->boundary = hunk->boundary; @@ -221,6 +225,10 @@ static git_blame_hunk *split_hunk_in_vector( new_line_count = hunk->lines_in_hunk - rel_line; nh = new_hunk((uint16_t)(hunk->final_start_line_number+rel_line), (uint16_t)new_line_count, (uint16_t)(hunk->orig_start_line_number+rel_line), hunk->orig_path); + + if (!nh) + return NULL; + git_oid_cpy(&nh->final_commit_id, &hunk->final_commit_id); git_oid_cpy(&nh->orig_commit_id, &hunk->orig_commit_id); @@ -270,6 +278,10 @@ static git_blame_hunk* hunk_from_entry(git_blame__entry *e) { git_blame_hunk *h = new_hunk( e->lno+1, e->num_lines, e->s_lno+1, e->suspect->path); + + if (!h) + return NULL; + git_oid_cpy(&h->final_commit_id, git_commit_id(e->suspect->commit)); git_oid_cpy(&h->orig_commit_id, git_commit_id(e->suspect->commit)); git_signature_dup(&h->final_signature, git_commit_author(e->suspect->commit)); @@ -307,6 +319,8 @@ static int blame_internal(git_blame *blame) blame->final_buf_size = git_blob_rawsize(blame->final_blob); ent = git__calloc(1, sizeof(git_blame__entry)); + GITERR_CHECK_ALLOC(ent); + ent->num_lines = index_blob_lines(blame); ent->lno = blame->options.min_line - 1; ent->num_lines = ent->num_lines - blame->options.min_line + 1; @@ -322,8 +336,9 @@ static int blame_internal(git_blame *blame) cleanup: for (ent = blame->ent; ent; ) { git_blame__entry *e = ent->next; + git_blame_hunk *h = hunk_from_entry(ent); - git_vector_insert(&blame->hunks, hunk_from_entry(ent)); + git_vector_insert(&blame->hunks, h); git_blame__free_entry(ent); ent = e; @@ -392,11 +407,14 @@ static int buffer_hunk_cb( if (!blame->current_hunk) { /* Line added at the end of the file */ blame->current_hunk = new_hunk(wedge_line, 0, wedge_line, blame->path); + GITERR_CHECK_ALLOC(blame->current_hunk); + git_vector_insert(&blame->hunks, blame->current_hunk); } else if (!hunk_starts_at_or_after_line(blame->current_hunk, wedge_line)){ /* If this hunk doesn't start between existing hunks, split a hunk up so it does */ blame->current_hunk = split_hunk_in_vector(&blame->hunks, blame->current_hunk, wedge_line - blame->current_hunk->orig_start_line_number, true); + GITERR_CHECK_ALLOC(blame->current_hunk); } return 0; @@ -425,6 +443,8 @@ static int buffer_line_cb( /* Create a new buffer-blame hunk with this line */ shift_hunks_by(&blame->hunks, blame->current_diff_line, 1); blame->current_hunk = new_hunk((uint16_t)blame->current_diff_line, 1, 0, blame->path); + GITERR_CHECK_ALLOC(blame->current_hunk); + git_vector_insert_sorted(&blame->hunks, blame->current_hunk, NULL); } blame->current_diff_line++; @@ -464,10 +484,14 @@ int git_blame_buffer( assert(out && reference && buffer && buffer_len); blame = git_blame__alloc(reference->repository, reference->options, reference->path); + GITERR_CHECK_ALLOC(blame); /* Duplicate all of the hunk structures in the reference blame */ git_vector_foreach(&reference->hunks, i, hunk) { - git_vector_insert(&blame->hunks, dup_hunk(hunk)); + git_blame_hunk *h = dup_hunk(hunk); + GITERR_CHECK_ALLOC(h); + + git_vector_insert(&blame->hunks, h); } /* Diff to the reference blob */ diff --git a/src/blame_git.c b/src/blame_git.c index 72afb852b..05aef5d99 100644 --- a/src/blame_git.c +++ b/src/blame_git.c @@ -35,10 +35,14 @@ static void origin_decref(git_blame__origin *o) /* Given a commit and a path in it, create a new origin structure. */ static int make_origin(git_blame__origin **out, git_commit *commit, const char *path) { - int error = 0; git_blame__origin *o; + size_t path_len = strlen(path); + int error = 0; - o = git__calloc(1, sizeof(*o) + strlen(path) + 1); + GITERR_CHECK_ALLOC_ADD(sizeof(*o), path_len); + GITERR_CHECK_ALLOC_ADD(sizeof(*o) + path_len, 1); + + o = git__calloc(1, sizeof(*o) + path_len + 1); GITERR_CHECK_ALLOC(o); o->commit = commit; o->refcnt = 1; diff --git a/src/buf_text.c b/src/buf_text.c index cb3661edb..ace54d725 100644 --- a/src/buf_text.c +++ b/src/buf_text.c @@ -29,6 +29,8 @@ int git_buf_text_puts_escaped( scan += count; } + GITERR_CHECK_ALLOC_ADD(buf->size, total); + GITERR_CHECK_ALLOC_ADD(buf->size + total, 1); if (git_buf_grow(buf, buf->size + total + 1) < 0) return -1; @@ -73,8 +75,10 @@ int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src) return git_buf_set(tgt, src->ptr, src->size); /* reduce reallocs while in the loop */ + GITERR_CHECK_ALLOC_ADD(src->size, 1); if (git_buf_grow(tgt, src->size + 1) < 0) return -1; + out = tgt->ptr; tgt->size = 0; @@ -117,13 +121,15 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) return git_buf_set(tgt, src->ptr, src->size); /* attempt to reduce reallocs while in the loop */ + GITERR_CHECK_ALLOC_ADD(src->size, src->size >> 4); + GITERR_CHECK_ALLOC_ADD(src->size + (src->size >> 4), 1); if (git_buf_grow(tgt, src->size + (src->size >> 4) + 1) < 0) return -1; tgt->size = 0; for (; next; scan = next + 1, next = memchr(scan, '\n', end - scan)) { size_t copylen = next - scan; - size_t needsize = tgt->size + copylen + 2 + 1; + size_t needsize; /* if we find mixed line endings, bail */ if (next > start && next[-1] == '\r') { @@ -131,6 +137,10 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) return GIT_PASSTHROUGH; } + GITERR_CHECK_ALLOC_ADD(tgt->size, copylen); + GITERR_CHECK_ALLOC_ADD(tgt->size + copylen, 3); + needsize = tgt->size + copylen + 3; + if (tgt->asize < needsize && git_buf_grow(tgt, needsize) < 0) return -1; diff --git a/src/buffer.c b/src/buffer.c index 8013457c5..ee8bba4ec 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -63,6 +63,14 @@ int git_buf_try_grow( /* round allocation up to multiple of 8 */ new_size = (new_size + 7) & ~7; + if (new_size < buf->size) { + if (mark_oom) + buf->ptr = git_buf__oom; + + giterr_set_oom(); + return -1; + } + new_ptr = git__realloc(new_ptr, new_size); if (!new_ptr) { @@ -131,6 +139,7 @@ int git_buf_set(git_buf *buf, const void *data, size_t len) git_buf_clear(buf); } else { if (data != buf->ptr) { + GITERR_CHECK_ALLOC_ADD(len, 1); ENSURE_SIZE(buf, len + 1); memmove(buf->ptr, data, len); } @@ -160,6 +169,7 @@ int git_buf_sets(git_buf *buf, const char *string) int git_buf_putc(git_buf *buf, char c) { + GITERR_CHECK_ALLOC_ADD(buf->size, 2); ENSURE_SIZE(buf, buf->size + 2); buf->ptr[buf->size++] = c; buf->ptr[buf->size] = '\0'; @@ -168,6 +178,8 @@ int git_buf_putc(git_buf *buf, char c) int git_buf_putcn(git_buf *buf, char c, size_t len) { + GITERR_CHECK_ALLOC_ADD(buf->size, len); + GITERR_CHECK_ALLOC_ADD(buf->size + len, 1); ENSURE_SIZE(buf, buf->size + len + 1); memset(buf->ptr + buf->size, c, len); buf->size += len; @@ -179,6 +191,8 @@ int git_buf_put(git_buf *buf, const char *data, size_t len) { if (len) { assert(data); + GITERR_CHECK_ALLOC_ADD(buf->size, len); + GITERR_CHECK_ALLOC_ADD(buf->size + len, 1); ENSURE_SIZE(buf, buf->size + len + 1); memmove(buf->ptr + buf->size, data, len); buf->size += len; @@ -201,8 +215,12 @@ int git_buf_encode_base64(git_buf *buf, const char *data, size_t len) size_t extra = len % 3; uint8_t *write, a, b, c; const uint8_t *read = (const uint8_t *)data; + size_t blocks = (len / 3) + !!extra; - ENSURE_SIZE(buf, buf->size + 4 * ((len / 3) + !!extra) + 1); + GITERR_CHECK_ALLOC_MULTIPLY(blocks, 4); + GITERR_CHECK_ALLOC_ADD(buf->size, 4 * blocks); + GITERR_CHECK_ALLOC_ADD(buf->size + 4 * blocks, 1); + ENSURE_SIZE(buf, buf->size + 4 * blocks + 1); write = (uint8_t *)&buf->ptr[buf->size]; /* convert each run of 3 bytes into 4 output bytes */ @@ -256,6 +274,8 @@ int git_buf_decode_base64(git_buf *buf, const char *base64, size_t len) size_t orig_size = buf->size; assert(len % 4 == 0); + GITERR_CHECK_ALLOC_ADD(buf->size, len / 4 * 3); + GITERR_CHECK_ALLOC_ADD(buf->size + (len / 4 * 3), 1); ENSURE_SIZE(buf, buf->size + (len / 4 * 3) + 1); for (i = 0; i < len; i += 4) { @@ -284,7 +304,12 @@ static const char b85str[] = int git_buf_encode_base85(git_buf *buf, const char *data, size_t len) { - ENSURE_SIZE(buf, buf->size + (5 * ((len / 4) + !!(len % 4))) + 1); + size_t blocks = (len / 4) + !!(len % 4); + + GITERR_CHECK_ALLOC_MULTIPLY(blocks, 5); + GITERR_CHECK_ALLOC_ADD(buf->size, 5 * blocks); + GITERR_CHECK_ALLOC_ADD(buf->size + 5 * blocks, 1); + ENSURE_SIZE(buf, buf->size + blocks * 5 + 1); while (len) { uint32_t acc = 0; @@ -317,8 +342,14 @@ int git_buf_encode_base85(git_buf *buf, const char *data, size_t len) int git_buf_vprintf(git_buf *buf, const char *format, va_list ap) { + size_t expected_size = strlen(format); int len; - const size_t expected_size = buf->size + (strlen(format) * 2); + + GITERR_CHECK_ALLOC_MULTIPLY(expected_size, 2); + expected_size *= 2; + + GITERR_CHECK_ALLOC_ADD(expected_size, buf->size); + expected_size += buf->size; ENSURE_SIZE(buf, expected_size); @@ -345,6 +376,8 @@ int git_buf_vprintf(git_buf *buf, const char *format, va_list ap) break; } + GITERR_CHECK_ALLOC_ADD(buf->size, len); + GITERR_CHECK_ALLOC_ADD(buf->size + len, 1); ENSURE_SIZE(buf, buf->size + len + 1); } @@ -481,6 +514,9 @@ int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...) /* expand buffer if needed */ if (total_size == 0) return 0; + + GITERR_CHECK_ALLOC_ADD(buf->size, total_size); + GITERR_CHECK_ALLOC_ADD(buf->size + total_size, 1); if (git_buf_grow(buf, buf->size + total_size + 1) < 0) return -1; @@ -559,6 +595,9 @@ int git_buf_join( if (str_a >= buf->ptr && str_a < buf->ptr + buf->size) offset_a = str_a - buf->ptr; + GITERR_CHECK_ALLOC_ADD(strlen_a, strlen_b); + GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b, need_sep); + GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b + need_sep, 1); if (git_buf_grow(buf, strlen_a + strlen_b + need_sep + 1) < 0) return -1; assert(buf->ptr); @@ -607,6 +646,11 @@ int git_buf_join3( sep_b = (str_b[len_b - 1] != separator); } + GITERR_CHECK_ALLOC_ADD(len_a, sep_a); + GITERR_CHECK_ALLOC_ADD(len_a + sep_a, len_b); + GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b, sep_b); + GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b, len_c); + GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b + len_c, 1); if (git_buf_grow(buf, len_a + sep_a + len_b + sep_b + len_c + 1) < 0) return -1; @@ -660,6 +704,8 @@ int git_buf_splice( const char *data, size_t nb_to_insert) { + size_t new_size; + assert(buf && where <= git_buf_len(buf) && where + nb_to_remove <= git_buf_len(buf)); @@ -667,7 +713,13 @@ int git_buf_splice( /* Ported from git.git * https://github.com/git/git/blob/16eed7c/strbuf.c#L159-176 */ - ENSURE_SIZE(buf, buf->size + nb_to_insert - nb_to_insert + 1); + new_size = buf->size - nb_to_remove; + + GITERR_CHECK_ALLOC_ADD(new_size, nb_to_insert); + new_size += nb_to_insert; + + GITERR_CHECK_ALLOC_ADD(new_size, 1); + ENSURE_SIZE(buf, new_size + 1); memmove(buf->ptr + where + nb_to_insert, buf->ptr + where + nb_to_remove, @@ -675,7 +727,7 @@ int git_buf_splice( memcpy(buf->ptr + where, data, nb_to_insert); - buf->size = buf->size + nb_to_insert - nb_to_remove; + buf->size = new_size; buf->ptr[buf->size] = '\0'; return 0; } diff --git a/src/common.h b/src/common.h index 4b4a99775..b53798eaf 100644 --- a/src/common.h +++ b/src/common.h @@ -174,6 +174,28 @@ GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int v GITERR_CHECK_VERSION(&(VERSION), _tmpl.version, #TYPE); \ memcpy((PTR), &_tmpl, sizeof(_tmpl)); } while (0) +/** Check for integer overflow from addition or multiplication */ +#define GIT_ALLOC_OVERFLOW_ADD(one, two) \ + ((one) + (two) < (one)) + +/** Check for integer overflow from multiplication */ +#define GIT_ALLOC_OVERFLOW_MULTIPLY(one, two) \ + (one && ((one) * (two)) / (one) != (two)) + +/** Check for additive overflow, failing if it would occur. */ +#define GITERR_CHECK_ALLOC_ADD(one, two) \ + if (GIT_ALLOC_OVERFLOW_ADD(one, two)) { \ + giterr_set_oom(); \ + return -1; \ + } + +/** Check for multiplicative overflow, failing if it would occur. */ +#define GITERR_CHECK_ALLOC_MULTIPLY(nelem, elsize) \ + if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize)) { \ + giterr_set_oom(); \ + return -1; \ + } + /* NOTE: other giterr functions are in the public errors.h header file */ #include "util.h" diff --git a/src/config_file.c b/src/config_file.c index 4f041e7e3..36f78563b 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -903,9 +903,11 @@ static char *reader_readline(struct reader *reader, bool skip_whitespace) line_len = line_end - line_src; - line = git__malloc(line_len + 1); - if (line == NULL) + if (GIT_ALLOC_OVERFLOW_ADD(line_len, 1) || + (line = git__malloc(line_len + 1)) == NULL) { + giterr_set_oom(); return NULL; + } memcpy(line, line_src, line_len); @@ -958,6 +960,8 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con int c, rpos; char *first_quote, *last_quote; git_buf buf = GIT_BUF_INIT; + size_t quoted_len, base_name_len = strlen(base_name); + /* * base_name is what came before the space. We should be at the * first quotation mark, except for now, line isn't being kept in @@ -966,13 +970,17 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con first_quote = strchr(line, '"'); last_quote = strrchr(line, '"'); + quoted_len = last_quote - first_quote; - if (last_quote - first_quote == 0) { + if (quoted_len == 0) { set_parse_error(reader, 0, "Missing closing quotation mark in section header"); return -1; } - git_buf_grow(&buf, strlen(base_name) + last_quote - first_quote + 2); + GITERR_CHECK_ALLOC_ADD(base_name_len, quoted_len); + GITERR_CHECK_ALLOC_ADD(base_name_len + quoted_len, 2); + + git_buf_grow(&buf, base_name_len + quoted_len + 2); git_buf_printf(&buf, "%s.", base_name); rpos = 0; @@ -1029,6 +1037,7 @@ static int parse_section_header(struct reader *reader, char **section_out) int name_length, c, pos; int result; char *line; + size_t line_len; line = reader_readline(reader, true); if (line == NULL) @@ -1042,7 +1051,10 @@ static int parse_section_header(struct reader *reader, char **section_out) return -1; } - name = (char *)git__malloc((size_t)(name_end - line) + 1); + line_len = (size_t)(name_end - line); + + GITERR_CHECK_ALLOC_ADD(line_len, 1); + name = git__malloc(line_len); GITERR_CHECK_ALLOC(name); name_length = 0; @@ -1603,11 +1615,16 @@ static char *escape_value(const char *ptr) /* '\"' -> '"' etc */ static char *fixup_line(const char *ptr, int quote_count) { - char *str = git__malloc(strlen(ptr) + 1); - char *out = str, *esc; + char *str, *out, *esc; + size_t ptr_len = strlen(ptr); - if (str == NULL) + if (GIT_ALLOC_OVERFLOW_ADD(ptr_len, 1) || + (str = git__malloc(ptr_len + 1)) == NULL) { + giterr_set_oom(); return NULL; + } + + out = str; while (*ptr != '\0') { if (*ptr == '"') { diff --git a/src/delta-apply.c b/src/delta-apply.c index a39c7af5c..e46c9631c 100644 --- a/src/delta-apply.c +++ b/src/delta-apply.c @@ -74,6 +74,7 @@ int git__delta_apply( return -1; } + GITERR_CHECK_ALLOC_ADD(res_sz, 1); res_dp = git__malloc(res_sz + 1); GITERR_CHECK_ALLOC(res_dp); diff --git a/src/delta.c b/src/delta.c index 8375a2c4d..f704fdfb1 100644 --- a/src/delta.c +++ b/src/delta.c @@ -119,6 +119,36 @@ struct git_delta_index { struct index_entry *hash[GIT_FLEX_ARRAY]; }; +static int lookup_index_alloc( + void **out, unsigned long *out_len, size_t entries, size_t hash_count) +{ + size_t entries_len, hash_len, + index_len = sizeof(struct git_delta_index); + + GITERR_CHECK_ALLOC_MULTIPLY(entries, sizeof(struct index_entry)); + entries_len = entries * sizeof(struct index_entry); + + GITERR_CHECK_ALLOC_ADD(index_len, entries_len); + index_len += entries_len; + + GITERR_CHECK_ALLOC_MULTIPLY(hash_count, sizeof(struct index_entry *)); + hash_len = hash_count * sizeof(struct index_entry *); + + GITERR_CHECK_ALLOC_ADD(index_len, hash_len); + index_len += hash_len; + + if (!git__is_ulong(index_len)) { + giterr_set_oom(); + return -1; + } + + *out = git__malloc(index_len); + GITERR_CHECK_ALLOC(*out); + + *out_len = index_len; + return 0; +} + struct git_delta_index * git_delta_create_index(const void *buf, unsigned long bufsize) { @@ -148,13 +178,9 @@ git_delta_create_index(const void *buf, unsigned long bufsize) hsize = 1 << i; hmask = hsize - 1; - /* allocate lookup index */ - memsize = sizeof(*index) + - sizeof(*hash) * hsize + - sizeof(*entry) * entries; - mem = git__malloc(memsize); - if (!mem) + if (lookup_index_alloc(&mem, &memsize, entries, hsize) < 0) return NULL; + index = mem; mem = index->hash; hash = mem; diff --git a/src/diff.c b/src/diff.c index e23d3891f..75e9ae9a3 100644 --- a/src/diff.c +++ b/src/diff.c @@ -1558,8 +1558,10 @@ int git_diff_format_email( goto on_error; } + GITERR_CHECK_ALLOC_ADD(offset, 1); summary = git__calloc(offset + 1, sizeof(char)); GITERR_CHECK_ALLOC(summary); + strncpy(summary, opts->summary, offset); } diff --git a/src/diff_driver.c b/src/diff_driver.c index c3c5f365b..67f1c591d 100644 --- a/src/diff_driver.c +++ b/src/diff_driver.c @@ -158,6 +158,29 @@ static git_diff_driver_registry *git_repository_driver_registry( return repo->diff_drivers; } +static int diff_driver_alloc( + git_diff_driver **out, size_t *namelen_out, const char *name) +{ + git_diff_driver *driver; + size_t driverlen = sizeof(git_diff_driver), + namelen = strlen(name); + + GITERR_CHECK_ALLOC_ADD(driverlen, namelen); + GITERR_CHECK_ALLOC_ADD(driverlen + namelen, 1); + + driver = git__calloc(1, driverlen + namelen + 1); + GITERR_CHECK_ALLOC(driver); + + memcpy(driver->name, name, namelen); + + *out = driver; + + if (namelen_out) + *namelen_out = namelen; + + return 0; +} + static int git_diff_driver_builtin( git_diff_driver **out, git_diff_driver_registry *reg, @@ -166,7 +189,7 @@ static int git_diff_driver_builtin( int error = 0; git_diff_driver_definition *ddef = NULL; git_diff_driver *drv = NULL; - size_t namelen, idx; + size_t idx; for (idx = 0; idx < ARRAY_SIZE(builtin_defs); ++idx) { if (!strcasecmp(driver_name, builtin_defs[idx].name)) { @@ -177,13 +200,10 @@ static int git_diff_driver_builtin( if (!ddef) goto done; - namelen = strlen(ddef->name); - - drv = git__calloc(1, sizeof(git_diff_driver) + namelen + 1); - GITERR_CHECK_ALLOC(drv); + if ((error = diff_driver_alloc(&drv, NULL, ddef->name)) < 0) + goto done; drv->type = DIFF_DRIVER_PATTERNLIST; - memcpy(drv->name, ddef->name, namelen); if (ddef->fns && (error = diff_driver_add_patterns( @@ -217,9 +237,9 @@ static int git_diff_driver_load( int error = 0; git_diff_driver_registry *reg; git_diff_driver *drv = NULL; - size_t namelen = strlen(driver_name); + size_t namelen; khiter_t pos; - git_config *cfg; + git_config *cfg = NULL; git_buf name = GIT_BUF_INIT; const git_config_entry *ce; bool found_driver = false; @@ -233,10 +253,10 @@ static int git_diff_driver_load( return 0; } - drv = git__calloc(1, sizeof(git_diff_driver) + namelen + 1); - GITERR_CHECK_ALLOC(drv); + if ((error = diff_driver_alloc(&drv, &namelen, driver_name)) < 0) + goto done; + drv->type = DIFF_DRIVER_AUTO; - memcpy(drv->name, driver_name, namelen); /* if you can't read config for repo, just use default driver */ if (git_repository_config_snapshot(&cfg, repo) < 0) { diff --git a/src/diff_patch.c b/src/diff_patch.c index a15107753..f5eecae66 100644 --- a/src/diff_patch.c +++ b/src/diff_patch.c @@ -388,8 +388,18 @@ static int diff_patch_with_delta_alloc( diff_patch_with_delta *pd; size_t old_len = *old_path ? strlen(*old_path) : 0; size_t new_len = *new_path ? strlen(*new_path) : 0; + size_t alloc_len = sizeof(*pd); - *out = pd = git__calloc(1, sizeof(*pd) + old_len + new_len + 2); + GITERR_CHECK_ALLOC_ADD(alloc_len, old_len); + alloc_len += old_len; + + GITERR_CHECK_ALLOC_ADD(alloc_len, new_len); + alloc_len += new_len; + + GITERR_CHECK_ALLOC_ADD(alloc_len, 2); + alloc_len += 2; + + *out = pd = git__calloc(1, alloc_len); GITERR_CHECK_ALLOC(pd); pd->patch.flags = GIT_DIFF_PATCH_ALLOCATED; diff --git a/src/diff_tform.c b/src/diff_tform.c index 9133a9b14..cad1356c3 100644 --- a/src/diff_tform.c +++ b/src/diff_tform.c @@ -831,6 +831,7 @@ int git_diff_find_similar( if ((opts.flags & GIT_DIFF_FIND_ALL) == 0) goto cleanup; + GITERR_CHECK_ALLOC_MULTIPLY(num_deltas, 2); sigcache = git__calloc(num_deltas * 2, sizeof(void *)); GITERR_CHECK_ALLOC(sigcache); diff --git a/src/filebuf.c b/src/filebuf.c index 25f6e52ef..1a9157558 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -271,6 +271,7 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode GITERR_CHECK_ALLOC(file->path_original); /* create the locking path by appending ".lock" to the original */ + GITERR_CHECK_ALLOC_ADD(path_len, GIT_FILELOCK_EXTLENGTH); file->path_lock = git__malloc(path_len + GIT_FILELOCK_EXTLENGTH); GITERR_CHECK_ALLOC(file->path_lock); @@ -437,8 +438,8 @@ int git_filebuf_printf(git_filebuf *file, const char *format, ...) } while ((size_t)len + 1 <= space_left); - tmp_buffer = git__malloc(len + 1); - if (!tmp_buffer) { + if (GIT_ALLOC_OVERFLOW_ADD(len, 1) || + !(tmp_buffer = git__malloc(len + 1))) { file->last_error = BUFERR_MEM; return -1; } diff --git a/src/fileops.c b/src/fileops.c index eddd5a804..eb24013e8 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -127,6 +127,7 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) git_buf_clear(buf); + GITERR_CHECK_ALLOC_ADD(len, 1); if (git_buf_grow(buf, len + 1) < 0) return -1; @@ -708,7 +709,10 @@ static int cp_link(const char *from, const char *to, size_t link_size) { int error = 0; ssize_t read_len; - char *link_data = git__malloc(link_size + 1); + char *link_data; + + GITERR_CHECK_ALLOC_ADD(link_size, 1); + link_data = git__malloc(link_size + 1); GITERR_CHECK_ALLOC(link_data); read_len = p_readlink(from, link_data, link_size); diff --git a/src/filter.c b/src/filter.c index 22eaf51a2..d5c669f01 100644 --- a/src/filter.c +++ b/src/filter.c @@ -228,7 +228,7 @@ int git_filter_register( const char *name, git_filter *filter, int priority) { git_filter_def *fdef; - size_t nattr = 0, nmatch = 0; + size_t nattr = 0, nmatch = 0, alloc_len; git_buf attrs = GIT_BUF_INIT; assert(name && filter); @@ -245,8 +245,16 @@ int git_filter_register( if (filter_def_scan_attrs(&attrs, &nattr, &nmatch, filter->attributes) < 0) return -1; - fdef = git__calloc( - sizeof(git_filter_def) + 2 * nattr * sizeof(char *), 1); + GITERR_CHECK_ALLOC_MULTIPLY(nattr, 2); + alloc_len = nattr * 2; + + GITERR_CHECK_ALLOC_MULTIPLY(alloc_len, sizeof(char *)); + alloc_len *= sizeof(char *); + + GITERR_CHECK_ALLOC_ADD(alloc_len, sizeof(git_filter_def)); + alloc_len += sizeof(git_filter_def); + + fdef = git__calloc(1, alloc_len); GITERR_CHECK_ALLOC(fdef); fdef->filter_name = git__strdup(name); @@ -379,6 +387,9 @@ static int filter_list_new( git_filter_list *fl = NULL; size_t pathlen = src->path ? strlen(src->path) : 0; + GITERR_CHECK_ALLOC_ADD(sizeof(git_filter_list), pathlen); + GITERR_CHECK_ALLOC_ADD(sizeof(git_filter_list) + pathlen, 1); + fl = git__calloc(1, sizeof(git_filter_list) + pathlen + 1); GITERR_CHECK_ALLOC(fl); diff --git a/src/index.c b/src/index.c index cbace3606..70090d12c 100644 --- a/src/index.c +++ b/src/index.c @@ -779,7 +779,9 @@ static int index_entry_create( return -1; } - entry = git__calloc(sizeof(struct entry_internal) + pathlen + 1, 1); + GITERR_CHECK_ALLOC_ADD(sizeof(struct entry_internal), pathlen); + GITERR_CHECK_ALLOC_ADD(sizeof(struct entry_internal) + pathlen, 1); + entry = git__calloc(1, sizeof(struct entry_internal) + pathlen + 1); GITERR_CHECK_ALLOC(entry); entry->pathlen = pathlen; @@ -826,9 +828,17 @@ static int index_entry_init( static git_index_reuc_entry *reuc_entry_alloc(const char *path) { - size_t pathlen = strlen(path); - struct reuc_entry_internal *entry = - git__calloc(sizeof(struct reuc_entry_internal) + pathlen + 1, 1); + size_t pathlen = strlen(path), + structlen = sizeof(struct reuc_entry_internal); + struct reuc_entry_internal *entry; + + if (GIT_ALLOC_OVERFLOW_ADD(structlen, pathlen) || + GIT_ALLOC_OVERFLOW_ADD(structlen + pathlen, 1)) { + giterr_set_oom(); + return NULL; + } + + entry = git__calloc(1, structlen + pathlen + 1); if (!entry) return NULL; diff --git a/src/iterator.c b/src/iterator.c index 196adddbf..e90cf30ff 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -336,7 +336,7 @@ static int tree_iterator__push_frame(tree_iterator *ti) { int error = 0; tree_iterator_frame *head = ti->head, *tf = NULL; - size_t i, n_entries = 0; + size_t i, n_entries = 0, alloclen; if (head->current >= head->n_entries || !head->entries[head->current]->tree) return GIT_ITEROVER; @@ -344,8 +344,13 @@ static int tree_iterator__push_frame(tree_iterator *ti) for (i = head->current; i < head->next; ++i) n_entries += git_tree_entrycount(head->entries[i]->tree); - tf = git__calloc(sizeof(tree_iterator_frame) + - n_entries * sizeof(tree_iterator_entry *), 1); + GITERR_CHECK_ALLOC_MULTIPLY(sizeof(tree_iterator_entry *), n_entries); + alloclen = sizeof(tree_iterator_entry *) * n_entries; + + GITERR_CHECK_ALLOC_ADD(alloclen, sizeof(tree_iterator_frame)); + alloclen += sizeof(tree_iterator_frame); + + tf = git__calloc(1, alloclen); GITERR_CHECK_ALLOC(tf); tf->n_entries = n_entries; diff --git a/src/merge.c b/src/merge.c index 7c38b5692..25d7bd7aa 100644 --- a/src/merge.c +++ b/src/merge.c @@ -1169,6 +1169,7 @@ int git_merge_diff_list__find_renames( goto done; if (diff_list->conflicts.length <= opts->target_limit) { + GITERR_CHECK_ALLOC_MULTIPLY(diff_list->conflicts.length, 3); cache_size = diff_list->conflicts.length * 3; cache = git__calloc(cache_size, sizeof(void *)); GITERR_CHECK_ALLOC(cache); @@ -2228,6 +2229,7 @@ static int merge_ancestor_head( assert(repo && our_head && their_heads); + GITERR_CHECK_ALLOC_ADD(their_heads_len, 1); oids = git__calloc(their_heads_len + 1, sizeof(git_oid)); GITERR_CHECK_ALLOC(oids); diff --git a/src/notes.c b/src/notes.c index f44c0bd95..1850e81c7 100644 --- a/src/notes.c +++ b/src/notes.c @@ -314,7 +314,7 @@ static int note_new( { git_note *note = NULL; - note = (git_note *)git__malloc(sizeof(git_note)); + note = git__malloc(sizeof(git_note)); GITERR_CHECK_ALLOC(note); git_oid_cpy(¬e->id, note_oid); diff --git a/src/odb.c b/src/odb.c index d2dff7178..80e02ef2b 100644 --- a/src/odb.c +++ b/src/odb.c @@ -233,6 +233,7 @@ int git_odb__hashlink(git_oid *out, const char *path) char *link_data; ssize_t read_len; + GITERR_CHECK_ALLOC_ADD(size, 1); link_data = git__malloc((size_t)(size + 1)); GITERR_CHECK_ALLOC(link_data); diff --git a/src/odb_loose.c b/src/odb_loose.c index ea9bdc4a0..e43b261fa 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -64,6 +64,8 @@ static int object_file_name( git_buf *name, const loose_backend *be, const git_oid *id) { /* expand length for object root + 40 hex sha1 chars + 2 * '/' + '\0' */ + GITERR_CHECK_ALLOC_ADD(be->objects_dirlen, GIT_OID_HEXSZ); + GITERR_CHECK_ALLOC_ADD(be->objects_dirlen + GIT_OID_HEXSZ, 3); if (git_buf_grow(name, be->objects_dirlen + GIT_OID_HEXSZ + 3) < 0) return -1; @@ -268,7 +270,8 @@ static void *inflate_tail(z_stream *s, void *hb, size_t used, obj_hdr *hdr) * initial sequence of inflated data from the tail of the * head buffer, if any. */ - if ((buf = git__malloc(hdr->size + 1)) == NULL) { + if (GIT_ALLOC_OVERFLOW_ADD(hdr->size, 1) || + (buf = git__malloc(hdr->size + 1)) == NULL) { inflateEnd(s); return NULL; } @@ -321,6 +324,7 @@ static int inflate_packlike_loose_disk_obj(git_rawobj *out, git_buf *obj) /* * allocate a buffer and inflate the data into it */ + GITERR_CHECK_ALLOC_ADD(hdr.size, 1); buf = git__malloc(hdr.size + 1); GITERR_CHECK_ALLOC(buf); @@ -520,6 +524,8 @@ static int locate_object_short_oid( int error; /* prealloc memory for OBJ_DIR/xx/xx..38x..xx */ + GITERR_CHECK_ALLOC_ADD(dir_len, GIT_OID_HEXSZ); + GITERR_CHECK_ALLOC_ADD(dir_len + GIT_OID_HEXSZ, 3); if (git_buf_grow(object_location, dir_len + 3 + GIT_OID_HEXSZ) < 0) return -1; @@ -563,6 +569,8 @@ static int locate_object_short_oid( return error; /* Update the location according to the oid obtained */ + GITERR_CHECK_ALLOC_ADD(dir_len, GIT_OID_HEXSZ); + GITERR_CHECK_ALLOC_ADD(dir_len + GIT_OID_HEXSZ, 2); git_buf_truncate(object_location, dir_len); if (git_buf_grow(object_location, dir_len + GIT_OID_HEXSZ + 2) < 0) @@ -928,6 +936,8 @@ int git_odb_backend_loose( objects_dirlen = strlen(objects_dir); + GITERR_CHECK_ALLOC_ADD(sizeof(loose_backend), objects_dirlen); + GITERR_CHECK_ALLOC_ADD(sizeof(loose_backend) + objects_dirlen, 2); backend = git__calloc(1, sizeof(loose_backend) + objects_dirlen + 2); GITERR_CHECK_ALLOC(backend); diff --git a/src/odb_mempack.c b/src/odb_mempack.c index d9b3a1824..a71d8db4b 100644 --- a/src/odb_mempack.c +++ b/src/odb_mempack.c @@ -47,6 +47,7 @@ static int impl__write(git_odb_backend *_backend, const git_oid *oid, const void if (rval == 0) return 0; + GITERR_CHECK_ALLOC_ADD(sizeof(struct memobject), len); obj = git__malloc(sizeof(struct memobject) + len); GITERR_CHECK_ALLOC(obj); diff --git a/src/oid.c b/src/oid.c index 5d29200b6..9aac47db6 100644 --- a/src/oid.c +++ b/src/oid.c @@ -261,6 +261,7 @@ struct git_oid_shorten { static int resize_trie(git_oid_shorten *self, size_t new_size) { + GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(trie_node)); self->nodes = git__realloc(self->nodes, new_size * sizeof(trie_node)); GITERR_CHECK_ALLOC(self->nodes); diff --git a/src/pack-objects.c b/src/pack-objects.c index 0040a826b..aea4770af 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -201,7 +201,11 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, return 0; if (pb->nr_objects >= pb->nr_alloc) { + GITERR_CHECK_ALLOC_ADD(pb->nr_alloc, 1024); + GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc + 1024, 3 / 2); pb->nr_alloc = (pb->nr_alloc + 1024) * 3 / 2; + + GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc, sizeof(*po)); pb->object_list = git__realloc(pb->object_list, pb->nr_alloc * sizeof(*po)); GITERR_CHECK_ALLOC(pb->object_list); @@ -499,8 +503,13 @@ static int cb_tag_foreach(const char *name, git_oid *oid, void *data) static git_pobject **compute_write_order(git_packbuilder *pb) { unsigned int i, wo_end, last_untagged; + git_pobject **wo; - git_pobject **wo = git__malloc(sizeof(*wo) * pb->nr_objects); + if (GIT_ALLOC_OVERFLOW_MULTIPLY(pb->nr_objects, sizeof(*wo)) || + (wo = git__malloc(pb->nr_objects * sizeof(*wo))) == NULL) { + giterr_set_oom(); + return NULL; + } for (i = 0; i < pb->nr_objects; i++) { git_pobject *po = pb->object_list + i; @@ -770,10 +779,13 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg, *mem_usage += sz; } if (!src->data) { - if (git_odb_read(&obj, pb->odb, &src_object->id) < 0) + size_t obj_sz; + + if (git_odb_read(&obj, pb->odb, &src_object->id) < 0 || + !git__is_ulong(obj_sz = git_odb_object_size(obj))) return -1; - sz = (unsigned long)git_odb_object_size(obj); + sz = (unsigned long)obj_sz; src->data = git__malloc(sz); GITERR_CHECK_ALLOC(src->data); memcpy(src->data, git_odb_object_data(obj), sz); @@ -817,7 +829,9 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg, trg_object->delta_data = NULL; } if (delta_cacheable(pb, src_size, trg_size, delta_size)) { + GITERR_CHECK_ALLOC_ADD(pb->delta_cache_size, delta_size); pb->delta_cache_size += delta_size; + git_packbuilder__cache_unlock(pb); trg_object->delta_data = git__realloc(delta_buf, delta_size); @@ -1088,6 +1102,7 @@ static int ll_find_deltas(git_packbuilder *pb, git_pobject **list, return 0; } + GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_threads, sizeof(*p)); p = git__malloc(pb->nr_threads * sizeof(*p)); GITERR_CHECK_ALLOC(p); @@ -1239,6 +1254,7 @@ static int prepare_pack(git_packbuilder *pb) if (pb->progress_cb) pb->progress_cb(GIT_PACKBUILDER_DELTAFICATION, 0, pb->nr_objects, pb->progress_cb_payload); + GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_objects, sizeof(*delta_list)); delta_list = git__malloc(pb->nr_objects * sizeof(*delta_list)); GITERR_CHECK_ALLOC(delta_list); diff --git a/src/pack.c b/src/pack.c index 47ce854c4..d475b28ee 100644 --- a/src/pack.c +++ b/src/pack.c @@ -683,8 +683,11 @@ int git_packfile_unpack( */ if (cached && stack_size == 1) { void *data = obj->data; + + GITERR_CHECK_ALLOC_ADD(obj->len, 1); obj->data = git__malloc(obj->len + 1); GITERR_CHECK_ALLOC(obj->data); + memcpy(obj->data, data, obj->len + 1); git_atomic_dec(&cached->refcount); goto cleanup; @@ -841,6 +844,7 @@ int packfile_unpack_compressed( z_stream stream; unsigned char *buffer, *in; + GITERR_CHECK_ALLOC_ADD(size, 1); buffer = git__calloc(1, size + 1); GITERR_CHECK_ALLOC(buffer); @@ -1092,6 +1096,9 @@ int git_packfile_alloc(struct git_pack_file **pack_out, const char *path) if (path_len < strlen(".idx")) return git_odb__error_notfound("invalid packfile path", NULL); + GITERR_CHECK_ALLOC_ADD(sizeof(*p), path_len); + GITERR_CHECK_ALLOC_ADD(sizeof(*p) + path_len, 2); + p = git__calloc(1, sizeof(*p) + path_len + 2); GITERR_CHECK_ALLOC(p); diff --git a/src/path.c b/src/path.c index 58d71921b..3dbd7187b 100644 --- a/src/path.c +++ b/src/path.c @@ -622,7 +622,9 @@ static bool _check_dir_contents( size_t sub_size = strlen(sub); /* leave base valid even if we could not make space for subdir */ - if (git_buf_try_grow(dir, dir_size + sub_size + 2, false, false) < 0) + if (GIT_ALLOC_OVERFLOW_ADD(dir_size, sub_size) || + GIT_ALLOC_OVERFLOW_ADD(dir_size + sub_size, 2) || + git_buf_try_grow(dir, dir_size + sub_size + 2, false, false) < 0) return false; /* save excursion */ @@ -822,6 +824,9 @@ int git_path_make_relative(git_buf *path, const char *parent) for (; (q = strchr(q, '/')) && *(q + 1); q++) depth++; + GITERR_CHECK_ALLOC_MULTIPLY(depth, 3); + GITERR_CHECK_ALLOC_ADD(depth * 3, plen); + GITERR_CHECK_ALLOC_ADD(depth * 3, 1); newlen = (depth * 3) + plen; /* save the offset as we might realllocate the pointer */ @@ -881,6 +886,7 @@ int git_path_iconv(git_path_iconv_t *ic, char **in, size_t *inlen) git_buf_clear(&ic->buf); while (1) { + GITERR_CHECK_ALLOC_ADD(wantlen, 1); if (git_buf_grow(&ic->buf, wantlen + 1) < 0) return -1; @@ -1057,6 +1063,45 @@ int git_path_direach( return error; } +static int entry_path_alloc( + char **out, + const char *path, + size_t path_len, + const char *de_path, + size_t de_len, + size_t alloc_extra) +{ + int need_slash = (path_len > 0 && path[path_len-1] != '/') ? 1 : 0; + size_t alloc_size = path_len; + char *entry_path; + + GITERR_CHECK_ALLOC_ADD(alloc_size, de_len); + alloc_size += de_len; + + GITERR_CHECK_ALLOC_ADD(alloc_size, need_slash); + alloc_size += need_slash; + + GITERR_CHECK_ALLOC_ADD(alloc_size, 1); + alloc_size++; + + GITERR_CHECK_ALLOC_ADD(alloc_size, alloc_extra); + alloc_size += alloc_extra; + + entry_path = git__calloc(1, alloc_size); + GITERR_CHECK_ALLOC(entry_path); + + if (path_len) + memcpy(entry_path, path, path_len); + + if (need_slash) + entry_path[path_len] = '/'; + + memcpy(&entry_path[path_len + need_slash], de_path, de_len); + + *out = entry_path; + return 0; +} + int git_path_dirload( const char *path, size_t prefix_len, @@ -1064,7 +1109,7 @@ int git_path_dirload( unsigned int flags, git_vector *contents) { - int error, need_slash; + int error; DIR *dir; size_t path_len; path_dirent_data de_data; @@ -1096,11 +1141,10 @@ int git_path_dirload( path += prefix_len; path_len -= prefix_len; - need_slash = (path_len > 0 && path[path_len-1] != '/') ? 1 : 0; while ((error = p_readdir_r(dir, de_buf, &de)) == 0 && de != NULL) { char *entry_path, *de_path = de->d_name; - size_t alloc_size, de_len = strlen(de_path); + size_t de_len = strlen(de_path); if (git_path_is_dot_or_dotdot(de_path)) continue; @@ -1110,17 +1154,9 @@ int git_path_dirload( break; #endif - alloc_size = path_len + need_slash + de_len + 1 + alloc_extra; - if ((entry_path = git__calloc(alloc_size, 1)) == NULL) { - error = -1; + if ((error = entry_path_alloc(&entry_path, + path, path_len, de_path, de_len, alloc_extra)) < 0) break; - } - - if (path_len) - memcpy(entry_path, path, path_len); - if (need_slash) - entry_path[path_len] = '/'; - memcpy(&entry_path[path_len + need_slash], de_path, de_len); if ((error = git_vector_insert(contents, entry_path)) < 0) { git__free(entry_path); diff --git a/src/pool.c b/src/pool.c index 7350c04c1..1599fc7a2 100644 --- a/src/pool.c +++ b/src/pool.c @@ -116,9 +116,11 @@ static void *pool_alloc_page(git_pool *pool, uint32_t size) pool->has_large_page_alloc = 1; } - page = git__calloc(1, alloc_size + sizeof(git_pool_page)); - if (!page) + if (GIT_ALLOC_OVERFLOW_ADD(alloc_size, sizeof(git_pool_page)) || + !(page = git__calloc(1, alloc_size + sizeof(git_pool_page)))) { + giterr_set_oom(); return NULL; + } page->size = alloc_size; page->avail = alloc_size - size; diff --git a/src/refs.c b/src/refs.c index 43c7333f2..33e931db5 100644 --- a/src/refs.c +++ b/src/refs.c @@ -37,10 +37,14 @@ enum { static git_reference *alloc_ref(const char *name) { git_reference *ref; - size_t namelen = strlen(name); + size_t namelen = strlen(name), reflen = sizeof(git_reference); - if ((ref = git__calloc(1, sizeof(git_reference) + namelen + 1)) == NULL) + if (GIT_ALLOC_OVERFLOW_ADD(reflen, namelen) || + GIT_ALLOC_OVERFLOW_ADD(reflen + namelen, 1) || + (ref = git__calloc(1, reflen + namelen + 1)) == NULL) { + giterr_set_oom(); return NULL; + } memcpy(ref->name, name, namelen + 1); @@ -94,10 +98,14 @@ git_reference *git_reference__set_name( git_reference *ref, const char *name) { size_t namelen = strlen(name); - git_reference *rewrite = - git__realloc(ref, sizeof(git_reference) + namelen + 1); - if (rewrite != NULL) + size_t reflen = sizeof(git_reference); + git_reference *rewrite = NULL; + + if (!GIT_ALLOC_OVERFLOW_ADD(reflen, namelen) && + !GIT_ALLOC_OVERFLOW_ADD(reflen + namelen, 1) && + (rewrite = git__realloc(ref, reflen + namelen + 1)) != NULL) memcpy(rewrite->name, name, namelen + 1); + return rewrite; } diff --git a/src/remote.c b/src/remote.c index 5ba7735ae..d96274f1d 100644 --- a/src/remote.c +++ b/src/remote.c @@ -383,10 +383,9 @@ int git_remote_lookup(git_remote **out, git_repository *repo, const char *name) if ((error = git_repository_config_snapshot(&config, repo)) < 0) return error; - remote = git__malloc(sizeof(git_remote)); + remote = git__calloc(1, sizeof(git_remote)); GITERR_CHECK_ALLOC(remote); - memset(remote, 0x0, sizeof(git_remote)); remote->update_fetchhead = 1; remote->name = git__strdup(name); GITERR_CHECK_ALLOC(remote->name); diff --git a/src/revwalk.c b/src/revwalk.c index e44385d48..2ba000c6b 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -503,13 +503,9 @@ static int prepare_walk(git_revwalk *walk) int git_revwalk_new(git_revwalk **revwalk_out, git_repository *repo) { - git_revwalk *walk; - - walk = git__malloc(sizeof(git_revwalk)); + git_revwalk *walk = git__calloc(1, sizeof(git_revwalk)); GITERR_CHECK_ALLOC(walk); - memset(walk, 0x0, sizeof(git_revwalk)); - walk->commits = git_oidmap_alloc(); GITERR_CHECK_ALLOC(walk->commits); diff --git a/src/sortedcache.c b/src/sortedcache.c index c6b226153..021f79632 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -15,7 +15,9 @@ int git_sortedcache_new( pathlen = path ? strlen(path) : 0; - sc = git__calloc(sizeof(git_sortedcache) + pathlen + 1, 1); + GITERR_CHECK_ALLOC_ADD(sizeof(git_sortedcache), pathlen); + GITERR_CHECK_ALLOC_ADD(sizeof(git_sortedcache) + pathlen, 1); + sc = git__calloc(1, sizeof(git_sortedcache) + pathlen + 1); GITERR_CHECK_ALLOC(sc); if (git_pool_init(&sc->pool, 1, 0) < 0 || diff --git a/src/tag.c b/src/tag.c index dc0827b8f..b82131401 100644 --- a/src/tag.c +++ b/src/tag.c @@ -117,6 +117,7 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) text_len = search - buffer; + GITERR_CHECK_ALLOC_ADD(text_len, 1); tag->tag_name = git__malloc(text_len + 1); GITERR_CHECK_ALLOC(tag->tag_name); @@ -141,6 +142,7 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) text_len = buffer_end - ++buffer; + GITERR_CHECK_ALLOC_ADD(text_len, 1); tag->message = git__malloc(text_len + 1); GITERR_CHECK_ALLOC(tag->message); diff --git a/src/transports/cred.c b/src/transports/cred.c index 1b4d29c0a..8e5447d18 100644 --- a/src/transports/cred.c +++ b/src/transports/cred.c @@ -311,6 +311,9 @@ int git_cred_username_new(git_cred **cred, const char *username) assert(cred); len = strlen(username); + + GITERR_CHECK_ALLOC_ADD(sizeof(git_cred_username), len); + GITERR_CHECK_ALLOC_ADD(sizeof(git_cred_username) + len, 1); c = git__malloc(sizeof(git_cred_username) + len + 1); GITERR_CHECK_ALLOC(c); diff --git a/src/transports/git.c b/src/transports/git.c index 6f25736b1..5ec98d867 100644 --- a/src/transports/git.c +++ b/src/transports/git.c @@ -154,7 +154,7 @@ static int git_proto_stream_alloc( if (!stream) return -1; - s = git__calloc(sizeof(git_proto_stream), 1); + s = git__calloc(1, sizeof(git_proto_stream)); GITERR_CHECK_ALLOC(s); s->parent.subtransport = &t->parent; @@ -347,7 +347,7 @@ int git_smart_subtransport_git(git_smart_subtransport **out, git_transport *owne if (!out) return -1; - t = git__calloc(sizeof(git_subtransport), 1); + t = git__calloc(1, sizeof(git_subtransport)); GITERR_CHECK_ALLOC(t); t->owner = owner; diff --git a/src/transports/local.c b/src/transports/local.c index c01755e34..89f2651cd 100644 --- a/src/transports/local.c +++ b/src/transports/local.c @@ -420,7 +420,7 @@ static int local_push( const git_error *last; char *ref = spec->refspec.dst; - status = git__calloc(sizeof(push_status), 1); + status = git__calloc(1, sizeof(push_status)); if (!status) goto on_error; diff --git a/src/transports/smart.c b/src/transports/smart.c index ec0ba3784..69b9d22cc 100644 --- a/src/transports/smart.c +++ b/src/transports/smart.c @@ -380,7 +380,7 @@ int git_transport_smart(git_transport **out, git_remote *owner, void *param) if (!param) return -1; - t = git__calloc(sizeof(transport_smart), 1); + t = git__calloc(1, sizeof(transport_smart)); GITERR_CHECK_ALLOC(t); t->parent.version = GIT_TRANSPORT_VERSION; diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index b5f9d6dbe..2f83e0d7b 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -103,6 +103,8 @@ static int comment_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_comment *pkt; + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_comment), len); + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_comment) + len, 1); pkt = git__malloc(sizeof(git_pkt_comment) + len + 1); GITERR_CHECK_ALLOC(pkt); @@ -122,6 +124,9 @@ static int err_pkt(git_pkt **out, const char *line, size_t len) /* Remove "ERR " from the line */ line += 4; len -= 4; + + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress), len); + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress) + len, 1); pkt = git__malloc(sizeof(git_pkt_err) + len + 1); GITERR_CHECK_ALLOC(pkt); @@ -141,6 +146,8 @@ static int data_pkt(git_pkt **out, const char *line, size_t len) line++; len--; + + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress), len); pkt = git__malloc(sizeof(git_pkt_data) + len); GITERR_CHECK_ALLOC(pkt); @@ -159,6 +166,8 @@ static int sideband_progress_pkt(git_pkt **out, const char *line, size_t len) line++; len--; + + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress), len); pkt = git__malloc(sizeof(git_pkt_progress) + len); GITERR_CHECK_ALLOC(pkt); @@ -177,6 +186,9 @@ static int sideband_error_pkt(git_pkt **out, const char *line, size_t len) line++; len--; + + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_err), len); + GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_err) + len, 1); pkt = git__malloc(sizeof(git_pkt_err) + len + 1); GITERR_CHECK_ALLOC(pkt); @@ -220,6 +232,7 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len) if (line[len - 1] == '\n') --len; + GITERR_CHECK_ALLOC_ADD(len, 1); pkt->head.name = git__malloc(len + 1); GITERR_CHECK_ALLOC(pkt->head.name); @@ -249,9 +262,13 @@ static int ok_pkt(git_pkt **out, const char *line, size_t len) pkt->type = GIT_PKT_OK; line += 3; /* skip "ok " */ - ptr = strchr(line, '\n'); + if (!(ptr = strchr(line, '\n'))) { + giterr_set(GITERR_NET, "Invalid packet line"); + return -1; + } len = ptr - line; + GITERR_CHECK_ALLOC_ADD(len, 1); pkt->ref = git__malloc(len + 1); GITERR_CHECK_ALLOC(pkt->ref); @@ -273,9 +290,13 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) pkt->type = GIT_PKT_NG; line += 3; /* skip "ng " */ - ptr = strchr(line, ' '); + if (!(ptr = strchr(line, ' '))) { + giterr_set(GITERR_NET, "Invalid packet line"); + return -1; + } len = ptr - line; + GITERR_CHECK_ALLOC_ADD(len, 1); pkt->ref = git__malloc(len + 1); GITERR_CHECK_ALLOC(pkt->ref); @@ -283,9 +304,13 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) pkt->ref[len] = '\0'; line = ptr + 1; - ptr = strchr(line, '\n'); + if (!(ptr = strchr(line, '\n'))) { + giterr_set(GITERR_NET, "Invalid packet line"); + return -1; + } len = ptr - line; + GITERR_CHECK_ALLOC_ADD(len, 1); pkt->msg = git__malloc(len + 1); GITERR_CHECK_ALLOC(pkt->msg); diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 5f1b99892..f023db4df 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -685,7 +685,7 @@ static int add_push_report_pkt(git_push *push, git_pkt *pkt) switch (pkt->type) { case GIT_PKT_OK: - status = git__calloc(sizeof(push_status), 1); + status = git__calloc(1, sizeof(push_status)); GITERR_CHECK_ALLOC(status); status->msg = NULL; status->ref = git__strdup(((git_pkt_ok *)pkt)->ref); @@ -696,7 +696,7 @@ static int add_push_report_pkt(git_push *push, git_pkt *pkt) } break; case GIT_PKT_NG: - status = git__calloc(sizeof(push_status), 1); + status = git__calloc(1, sizeof(push_status)); GITERR_CHECK_ALLOC(status); status->ref = git__strdup(((git_pkt_ng *)pkt)->ref); status->msg = git__strdup(((git_pkt_ng *)pkt)->msg); diff --git a/src/transports/winhttp.c b/src/transports/winhttp.c index 56b7c99f9..278ef22c4 100644 --- a/src/transports/winhttp.c +++ b/src/transports/winhttp.c @@ -1178,7 +1178,7 @@ static int winhttp_stream_alloc(winhttp_subtransport *t, winhttp_stream **stream if (!stream) return -1; - s = git__calloc(sizeof(winhttp_stream), 1); + s = git__calloc(1, sizeof(winhttp_stream)); GITERR_CHECK_ALLOC(s); s->parent.subtransport = &t->parent; @@ -1329,7 +1329,7 @@ int git_smart_subtransport_http(git_smart_subtransport **out, git_transport *own if (!out) return -1; - t = git__calloc(sizeof(winhttp_subtransport), 1); + t = git__calloc(1, sizeof(winhttp_subtransport)); GITERR_CHECK_ALLOC(t); t->owner = (transport_smart *)owner; diff --git a/src/tree.c b/src/tree.c index 9693f4eca..2c8b89291 100644 --- a/src/tree.c +++ b/src/tree.c @@ -84,11 +84,15 @@ int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2) static git_tree_entry *alloc_entry(const char *filename) { git_tree_entry *entry = NULL; - size_t filename_len = strlen(filename); + size_t filename_len = strlen(filename), + tree_len = sizeof(git_tree_entry); - entry = git__malloc(sizeof(git_tree_entry) + filename_len + 1); - if (!entry) + if (GIT_ALLOC_OVERFLOW_ADD(tree_len, filename_len) || + GIT_ALLOC_OVERFLOW_ADD(tree_len + filename_len, 1) || + !(entry = git__malloc(tree_len + filename_len + 1))) { + giterr_set_oom(); return NULL; + } memset(entry, 0x0, sizeof(git_tree_entry)); memcpy(entry->filename, filename, filename_len); @@ -205,12 +209,16 @@ void git_tree_entry_free(git_tree_entry *entry) int git_tree_entry_dup(git_tree_entry **dest, const git_tree_entry *source) { - size_t total_size; + size_t total_size = sizeof(git_tree_entry); git_tree_entry *copy; assert(source); - total_size = sizeof(git_tree_entry) + source->filename_len + 1; + GITERR_CHECK_ALLOC_ADD(total_size, source->filename_len); + total_size += source->filename_len; + + GITERR_CHECK_ALLOC_ADD(total_size, 1); + total_size++; copy = git__malloc(total_size); GITERR_CHECK_ALLOC(copy); diff --git a/src/tsort.c b/src/tsort.c index 4885e435b..b92e52056 100644 --- a/src/tsort.c +++ b/src/tsort.c @@ -184,7 +184,10 @@ static int check_invariant(struct tsort_run *stack, ssize_t stack_curr) static int resize(struct tsort_store *store, size_t new_size) { if (store->alloc < new_size) { - void **tempstore = git__realloc(store->storage, new_size * sizeof(void *)); + void **tempstore; + + GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(void *)); + tempstore = git__realloc(store->storage, new_size * sizeof(void *)); /** * Do not propagate on OOM; this will abort the sort and diff --git a/src/util.h b/src/util.h index 89816a8c9..6c94a5aa0 100644 --- a/src/util.h +++ b/src/util.h @@ -64,7 +64,12 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n) length = p_strnlen(str, n); - ptr = (char*)git__malloc(length + 1); + if (GIT_ALLOC_OVERFLOW_ADD(length, 1)) { + giterr_set_oom(); + return NULL; + } + + ptr = git__malloc(length + 1); if (!ptr) return NULL; @@ -80,7 +85,13 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n) /* NOTE: This doesn't do null or '\0' checking. Watch those boundaries! */ GIT_INLINE(char *) git__substrdup(const char *start, size_t n) { - char *ptr = (char*)git__malloc(n+1); + char *ptr; + + if (GIT_ALLOC_OVERFLOW_ADD(n, 1) || !(ptr = git__malloc(n+1))) { + giterr_set_oom(); + return NULL; + } + memcpy(ptr, start, n); ptr[n] = '\0'; return ptr; diff --git a/src/vector.c b/src/vector.c index c769b696a..b636032b1 100644 --- a/src/vector.c +++ b/src/vector.c @@ -29,12 +29,12 @@ GIT_INLINE(size_t) compute_new_size(git_vector *v) GIT_INLINE(int) resize_vector(git_vector *v, size_t new_size) { - size_t new_bytes = new_size * sizeof(void *); + size_t new_bytes; void *new_contents; /* Check for overflow */ - if (new_bytes / sizeof(void *) != new_size) - GITERR_CHECK_ALLOC(NULL); + GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(void *)); + new_bytes = new_size * sizeof(void *); new_contents = git__realloc(v->contents, new_bytes); GITERR_CHECK_ALLOC(new_contents); @@ -51,6 +51,7 @@ int git_vector_dup(git_vector *v, const git_vector *src, git_vector_cmp cmp) assert(v && src); + GITERR_CHECK_ALLOC_MULTIPLY(src->length, sizeof(void *)); bytes = src->length * sizeof(void *); v->_alloc_size = src->length; diff --git a/src/win32/dir.c b/src/win32/dir.c index c7427ea54..9953289f6 100644 --- a/src/win32/dir.c +++ b/src/win32/dir.c @@ -18,9 +18,13 @@ git__DIR *git__opendir(const char *dir) dirlen = strlen(dir); - new = git__calloc(sizeof(*new) + dirlen + 1, 1); - if (!new) + if (GIT_ALLOC_OVERFLOW_ADD(sizeof(*new), dirlen) || + GIT_ALLOC_OVERFLOW_ADD(sizeof(*new) + dirlen, 1) || + !(new = git__calloc(1, sizeof(*new) + dirlen + 1))) { + giterr_set_oom(); return NULL; + } + memcpy(new->dir, dir, dirlen); new->h = FindFirstFileW(filter_w, &new->f); diff --git a/src/win32/utf-conv.c b/src/win32/utf-conv.c index b0205b019..624611205 100644 --- a/src/win32/utf-conv.c +++ b/src/win32/utf-conv.c @@ -99,9 +99,8 @@ int git__utf8_to_16_alloc(wchar_t **dest, const char *src) return -1; } - *dest = git__malloc(utf16_size * sizeof(wchar_t)); - - if (!*dest) { + if (GIT_ALLOC_OVERFLOW_MULTIPLY(utf16_size, sizeof(wchar_t)) || + !(*dest = git__malloc(utf16_size * sizeof(wchar_t)))) { errno = ENOMEM; return -1; } diff --git a/src/zstream.c b/src/zstream.c index e75fb265e..06660e981 100644 --- a/src/zstream.c +++ b/src/zstream.c @@ -134,6 +134,7 @@ int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len) while (!git_zstream_done(&zs)) { size_t step = git_zstream_suggest_output_len(&zs), written; + GITERR_CHECK_ALLOC_ADD(out->size, step); if ((error = git_buf_grow(out, out->size + step)) < 0) goto done; diff --git a/tests/core/errors.c b/tests/core/errors.c index 366d8f16a..b6364100b 100644 --- a/tests/core/errors.c +++ b/tests/core/errors.c @@ -109,3 +109,43 @@ void test_core_errors__restore(void) cl_assert_equal_i(42, giterr_last()->klass); cl_assert_equal_s("Foo: bar", giterr_last()->message); } + +static int test_arraysize_multiply(size_t nelem, size_t size) +{ + GITERR_CHECK_ALLOC_MULTIPLY(nelem, size); + return 0; +} + +void test_core_errors__integer_overflow_alloc_multiply(void) +{ + cl_git_pass(test_arraysize_multiply(10, 10)); + cl_git_pass(test_arraysize_multiply(1000, 1000)); + cl_git_pass(test_arraysize_multiply(SIZE_MAX/sizeof(void *), sizeof(void *))); + cl_git_pass(test_arraysize_multiply(0, 10)); + cl_git_pass(test_arraysize_multiply(10, 0)); + + cl_git_fail(test_arraysize_multiply(SIZE_MAX-1, sizeof(void *))); + cl_git_fail(test_arraysize_multiply((SIZE_MAX/sizeof(void *))+1, sizeof(void *))); + + cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass); + cl_assert_equal_s("Out of memory", giterr_last()->message); +} + +static int test_arraysize_add(size_t one, size_t two) +{ + GITERR_CHECK_ALLOC_ADD(one, two); + return 0; +} + +void test_core_errors__integer_overflow_alloc_add(void) +{ + cl_git_pass(test_arraysize_add(10, 10)); + cl_git_pass(test_arraysize_add(1000, 1000)); + cl_git_pass(test_arraysize_add(SIZE_MAX-10, 10)); + + cl_git_fail(test_arraysize_multiply(SIZE_MAX-1, 2)); + cl_git_fail(test_arraysize_multiply(SIZE_MAX, SIZE_MAX)); + + cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass); + cl_assert_equal_s("Out of memory", giterr_last()->message); +} From 15d54fdd345dadc2854200ce5b9aad0949e3949b Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 10 Feb 2015 22:34:03 -0500 Subject: [PATCH 02/15] odb__hashlink: check st.st_size before casting --- src/odb.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/odb.c b/src/odb.c index 80e02ef2b..fcb21c1ce 100644 --- a/src/odb.c +++ b/src/odb.c @@ -216,28 +216,28 @@ int git_odb__hashfd_filtered( int git_odb__hashlink(git_oid *out, const char *path) { struct stat st; - git_off_t size; + size_t size; int result; if (git_path_lstat(path, &st) < 0) return -1; - size = st.st_size; - - if (!git__is_sizet(size)) { - giterr_set(GITERR_OS, "File size overflow for 32-bit systems"); + if (!git__is_sizet(st.st_size)) { + giterr_set(GITERR_FILESYSTEM, "File size overflow for 32-bit systems"); return -1; } + size = (size_t)st.st_size; + if (S_ISLNK(st.st_mode)) { char *link_data; ssize_t read_len; GITERR_CHECK_ALLOC_ADD(size, 1); - link_data = git__malloc((size_t)(size + 1)); + link_data = git__malloc(size + 1); GITERR_CHECK_ALLOC(link_data); - read_len = p_readlink(path, link_data, (size_t)size); + read_len = p_readlink(path, link_data, size); link_data[size] = '\0'; if (read_len != (ssize_t)size) { giterr_set(GITERR_OS, "Failed to read symlink data for '%s'", path); @@ -245,13 +245,13 @@ int git_odb__hashlink(git_oid *out, const char *path) return -1; } - result = git_odb_hash(out, link_data, (size_t)size, GIT_OBJ_BLOB); + result = git_odb_hash(out, link_data, size, GIT_OBJ_BLOB); git__free(link_data); } else { int fd = git_futils_open_ro(path); if (fd < 0) return -1; - result = git_odb__hashfd(out, fd, (size_t)size, GIT_OBJ_BLOB); + result = git_odb__hashfd(out, fd, size, GIT_OBJ_BLOB); p_close(fd); } From 3603cb0978b7ef21ff9cd63693ebd6d27bc2bc53 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 10 Feb 2015 23:13:49 -0500 Subject: [PATCH 03/15] git__*allocarray: safer realloc and malloc Introduce git__reallocarray that checks the product of the number of elements and element size for overflow before allocation. Also introduce git__mallocarray that behaves like calloc, but without the `c`. (It does not zero memory, for those truly worried about every cycle.) --- src/array.h | 3 +-- src/oid.c | 3 +-- src/pack-objects.c | 14 +++++--------- src/tsort.c | 3 +-- src/util.h | 22 ++++++++++++++++++++++ src/vector.c | 7 +------ src/win32/utf-conv.c | 3 +-- 7 files changed, 32 insertions(+), 23 deletions(-) diff --git a/src/array.h b/src/array.h index ca5a35ffe..0d0795370 100644 --- a/src/array.h +++ b/src/array.h @@ -57,8 +57,7 @@ GIT_INLINE(void *) git_array_grow(void *_a, size_t item_size) new_size = a->size * 3 / 2; } - if (GIT_ALLOC_OVERFLOW_MULTIPLY(new_size, item_size) || - (new_array = git__realloc(a->ptr, new_size * item_size)) == NULL) + if ((new_array = git__reallocarray(a->ptr, new_size, item_size)) == NULL) goto on_oom; a->ptr = new_array; a->asize = new_size; a->size++; diff --git a/src/oid.c b/src/oid.c index 9aac47db6..9fe2ebb65 100644 --- a/src/oid.c +++ b/src/oid.c @@ -261,8 +261,7 @@ struct git_oid_shorten { static int resize_trie(git_oid_shorten *self, size_t new_size) { - GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(trie_node)); - self->nodes = git__realloc(self->nodes, new_size * sizeof(trie_node)); + self->nodes = git__reallocarray(self->nodes, new_size, sizeof(trie_node)); GITERR_CHECK_ALLOC(self->nodes); if (new_size > self->size) { diff --git a/src/pack-objects.c b/src/pack-objects.c index aea4770af..cd0deb29a 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -205,9 +205,8 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc + 1024, 3 / 2); pb->nr_alloc = (pb->nr_alloc + 1024) * 3 / 2; - GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc, sizeof(*po)); - pb->object_list = git__realloc(pb->object_list, - pb->nr_alloc * sizeof(*po)); + pb->object_list = git__reallocarray(pb->object_list, + pb->nr_alloc, sizeof(*po)); GITERR_CHECK_ALLOC(pb->object_list); rehash(pb); } @@ -505,8 +504,7 @@ static git_pobject **compute_write_order(git_packbuilder *pb) unsigned int i, wo_end, last_untagged; git_pobject **wo; - if (GIT_ALLOC_OVERFLOW_MULTIPLY(pb->nr_objects, sizeof(*wo)) || - (wo = git__malloc(pb->nr_objects * sizeof(*wo))) == NULL) { + if ((wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL) { giterr_set_oom(); return NULL; } @@ -1102,8 +1100,7 @@ static int ll_find_deltas(git_packbuilder *pb, git_pobject **list, return 0; } - GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_threads, sizeof(*p)); - p = git__malloc(pb->nr_threads * sizeof(*p)); + p = git__mallocarray(pb->nr_threads, sizeof(*p)); GITERR_CHECK_ALLOC(p); /* Partition the work among the threads */ @@ -1254,8 +1251,7 @@ static int prepare_pack(git_packbuilder *pb) if (pb->progress_cb) pb->progress_cb(GIT_PACKBUILDER_DELTAFICATION, 0, pb->nr_objects, pb->progress_cb_payload); - GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_objects, sizeof(*delta_list)); - delta_list = git__malloc(pb->nr_objects * sizeof(*delta_list)); + delta_list = git__mallocarray(pb->nr_objects, sizeof(*delta_list)); GITERR_CHECK_ALLOC(delta_list); for (i = 0; i < pb->nr_objects; ++i) { diff --git a/src/tsort.c b/src/tsort.c index b92e52056..e59819204 100644 --- a/src/tsort.c +++ b/src/tsort.c @@ -186,8 +186,7 @@ static int resize(struct tsort_store *store, size_t new_size) if (store->alloc < new_size) { void **tempstore; - GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(void *)); - tempstore = git__realloc(store->storage, new_size * sizeof(void *)); + tempstore = git__reallocarray(store->storage, new_size, sizeof(void *)); /** * Do not propagate on OOM; this will abort the sort and diff --git a/src/util.h b/src/util.h index 6c94a5aa0..7693ef809 100644 --- a/src/util.h +++ b/src/util.h @@ -104,6 +104,28 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size) return new_ptr; } +/** + * Similar to `git__realloc`, except that it is suitable for reallocing an + * array to a new number of elements of `nelem`, each of size `elsize`. + * The total size calculation is checked for overflow. + */ +GIT_INLINE(void *) git__reallocarray(void *ptr, size_t nelem, size_t elsize) +{ + void *new_ptr = NULL; + if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize) || + !(new_ptr = realloc(ptr, nelem * elsize))) + giterr_set_oom(); + return new_ptr; +} + +/** + * Similar to `git__calloc`, except that it does not zero memory. + */ +GIT_INLINE(void *) git__mallocarray(size_t nelem, size_t elsize) +{ + return git__reallocarray(NULL, nelem, elsize); +} + GIT_INLINE(void) git__free(void *ptr) { free(ptr); diff --git a/src/vector.c b/src/vector.c index b636032b1..27eafebc8 100644 --- a/src/vector.c +++ b/src/vector.c @@ -29,14 +29,9 @@ GIT_INLINE(size_t) compute_new_size(git_vector *v) GIT_INLINE(int) resize_vector(git_vector *v, size_t new_size) { - size_t new_bytes; void *new_contents; - /* Check for overflow */ - GITERR_CHECK_ALLOC_MULTIPLY(new_size, sizeof(void *)); - new_bytes = new_size * sizeof(void *); - - new_contents = git__realloc(v->contents, new_bytes); + new_contents = git__reallocarray(v->contents, new_size, sizeof(void *)); GITERR_CHECK_ALLOC(new_contents); v->_alloc_size = new_size; diff --git a/src/win32/utf-conv.c b/src/win32/utf-conv.c index 624611205..0dad4eab0 100644 --- a/src/win32/utf-conv.c +++ b/src/win32/utf-conv.c @@ -99,8 +99,7 @@ int git__utf8_to_16_alloc(wchar_t **dest, const char *src) return -1; } - if (GIT_ALLOC_OVERFLOW_MULTIPLY(utf16_size, sizeof(wchar_t)) || - !(*dest = git__malloc(utf16_size * sizeof(wchar_t)))) { + if (!(*dest = git__mallocarray(utf16_size, sizeof(wchar_t)))) { errno = ENOMEM; return -1; } From 4aa664ae3953d99c2ae4cd769f02818bc122eebc Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 10 Feb 2015 23:55:07 -0500 Subject: [PATCH 04/15] git_buf_grow_by: increase buf asize incrementally Introduce `git_buf_grow_by` to incrementally increase the size of a `git_buf`, performing an overflow calculation on the growth. --- src/buf_text.c | 13 ++++--------- src/buffer.c | 17 ++++++++++++++--- src/buffer.h | 12 ++++++++++++ src/transports/smart_pkt.c | 2 +- src/zstream.c | 3 +-- tests/buf/basic.c | 20 ++++++++++++++++++++ tests/buf/oom.c | 10 ++++++++++ 7 files changed, 62 insertions(+), 15 deletions(-) diff --git a/src/buf_text.c b/src/buf_text.c index ace54d725..08b86f4cc 100644 --- a/src/buf_text.c +++ b/src/buf_text.c @@ -29,9 +29,8 @@ int git_buf_text_puts_escaped( scan += count; } - GITERR_CHECK_ALLOC_ADD(buf->size, total); - GITERR_CHECK_ALLOC_ADD(buf->size + total, 1); - if (git_buf_grow(buf, buf->size + total + 1) < 0) + GITERR_CHECK_ALLOC_ADD(total, 1); + if (git_buf_grow_by(buf, total + 1) < 0) return -1; for (scan = string; *scan; ) { @@ -129,7 +128,6 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) for (; next; scan = next + 1, next = memchr(scan, '\n', end - scan)) { size_t copylen = next - scan; - size_t needsize; /* if we find mixed line endings, bail */ if (next > start && next[-1] == '\r') { @@ -137,11 +135,8 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) return GIT_PASSTHROUGH; } - GITERR_CHECK_ALLOC_ADD(tgt->size, copylen); - GITERR_CHECK_ALLOC_ADD(tgt->size + copylen, 3); - needsize = tgt->size + copylen + 3; - - if (tgt->asize < needsize && git_buf_grow(tgt, needsize) < 0) + GITERR_CHECK_ALLOC_ADD(copylen, 3); + if (git_buf_grow_by(tgt, copylen + 3) < 0) return -1; if (next > scan) { diff --git a/src/buffer.c b/src/buffer.c index ee8bba4ec..8098bb1e7 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -101,6 +101,18 @@ int git_buf_grow(git_buf *buffer, size_t target_size) return git_buf_try_grow(buffer, target_size, true, true); } +int git_buf_grow_by(git_buf *buffer, size_t additional_size) +{ + if (GIT_ALLOC_OVERFLOW_ADD(buffer->size, additional_size)) { + buffer->ptr = git_buf__oom; + giterr_set_oom(); + return -1; + } + + return git_buf_try_grow( + buffer, buffer->size + additional_size, true, true); +} + void git_buf_free(git_buf *buf) { if (!buf) return; @@ -515,9 +527,8 @@ int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...) if (total_size == 0) return 0; - GITERR_CHECK_ALLOC_ADD(buf->size, total_size); - GITERR_CHECK_ALLOC_ADD(buf->size + total_size, 1); - if (git_buf_grow(buf, buf->size + total_size + 1) < 0) + GITERR_CHECK_ALLOC_ADD(total_size, 1); + if (git_buf_grow_by(buf, total_size + 1) < 0) return -1; out = buf->ptr + buf->size; diff --git a/src/buffer.h b/src/buffer.h index 8ee4b532c..52342e309 100644 --- a/src/buffer.h +++ b/src/buffer.h @@ -36,6 +36,18 @@ GIT_INLINE(bool) git_buf_is_allocated(const git_buf *buf) */ extern void git_buf_init(git_buf *buf, size_t initial_size); +/** + * Resize the buffer allocation to make more space. + * + * This will attempt to grow the buffer to accommodate the additional size. + * It is similar to `git_buf_grow`, but performs the new size calculation, + * checking for overflow. + * + * Like `git_buf_grow`, if this is a user-supplied buffer, this will allocate + * a new buffer. + */ +extern int git_buf_grow_by(git_buf *buffer, size_t additional_size); + /** * Attempt to grow the buffer to hold at least `target_size` bytes. * diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index 2f83e0d7b..ad81ad1b0 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -513,7 +513,7 @@ static int buffer_want_with_caps(const git_remote_head *head, transport_smart_ca len = (unsigned int) (strlen("XXXXwant ") + GIT_OID_HEXSZ + 1 /* NUL */ + git_buf_len(&str) + 1 /* LF */); - git_buf_grow(buf, git_buf_len(buf) + len); + git_buf_grow_by(buf, len); git_oid_fmt(oid, &head->oid); git_buf_printf(buf, "%04xwant %s %s\n", len, oid, git_buf_cstr(&str)); git_buf_free(&str); diff --git a/src/zstream.c b/src/zstream.c index 06660e981..2130bc3ca 100644 --- a/src/zstream.c +++ b/src/zstream.c @@ -134,8 +134,7 @@ int git_zstream_deflatebuf(git_buf *out, const void *in, size_t in_len) while (!git_zstream_done(&zs)) { size_t step = git_zstream_suggest_output_len(&zs), written; - GITERR_CHECK_ALLOC_ADD(out->size, step); - if ((error = git_buf_grow(out, out->size + step)) < 0) + if ((error = git_buf_grow_by(out, step)) < 0) goto done; written = out->asize - out->size; diff --git a/tests/buf/basic.c b/tests/buf/basic.c index d558757a9..dcc0916d2 100644 --- a/tests/buf/basic.c +++ b/tests/buf/basic.c @@ -15,6 +15,26 @@ void test_buf_basic__resize(void) git_buf_free(&buf1); } +void test_buf_basic__resize_incremental(void) +{ + git_buf buf1 = GIT_BUF_INIT; + + /* Presently, asking for 6 bytes will round up to 8. */ + cl_git_pass(git_buf_puts(&buf1, "Hello")); + cl_assert_equal_i(5, buf1.size); + cl_assert_equal_i(8, buf1.asize); + + /* Ensure an additional byte does not realloc. */ + cl_git_pass(git_buf_grow_by(&buf1, 1)); + cl_assert_equal_i(5, buf1.size); + cl_assert_equal_i(8, buf1.asize); + + /* But requesting many does. */ + cl_git_pass(git_buf_grow_by(&buf1, 16)); + cl_assert_equal_i(5, buf1.size); + cl_assert(buf1.asize > 8); +} + void test_buf_basic__printf(void) { git_buf buf2 = GIT_BUF_INIT; diff --git a/tests/buf/oom.c b/tests/buf/oom.c index 709439aa5..b9fd29cbb 100644 --- a/tests/buf/oom.c +++ b/tests/buf/oom.c @@ -29,3 +29,13 @@ void test_buf_oom__grow(void) git_buf_free(&buf); } + +void test_buf_oom__grow_by(void) +{ + git_buf buf = GIT_BUF_INIT; + + buf.size = SIZE_MAX-10; + + cl_assert(git_buf_grow_by(&buf, 50) == -1); + cl_assert(git_buf_oom(&buf)); +} From 2884cc42de8b20a58cec8488d014a853d47c047e Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 11 Feb 2015 09:39:38 -0500 Subject: [PATCH 05/15] overflow checking: don't make callers set oom Have the ALLOC_OVERFLOW testing macros also simply set_oom in the case where a computation would overflow, so that callers don't need to. --- src/buffer.c | 1 - src/common.h | 14 ++++---------- src/config_file.c | 2 -- src/delta.c | 2 +- src/index.c | 4 +--- src/pack-objects.c | 4 +--- src/pool.c | 4 +--- src/refs.c | 4 +--- src/tree.c | 4 +--- src/util.h | 15 ++++----------- src/win32/dir.c | 4 +--- tests/core/errors.c | 21 +++++++++++++++++++++ 12 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index 8098bb1e7..0d0314439 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -105,7 +105,6 @@ int git_buf_grow_by(git_buf *buffer, size_t additional_size) { if (GIT_ALLOC_OVERFLOW_ADD(buffer->size, additional_size)) { buffer->ptr = git_buf__oom; - giterr_set_oom(); return -1; } diff --git a/src/common.h b/src/common.h index b53798eaf..8b7d6936d 100644 --- a/src/common.h +++ b/src/common.h @@ -176,25 +176,19 @@ GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int v /** Check for integer overflow from addition or multiplication */ #define GIT_ALLOC_OVERFLOW_ADD(one, two) \ - ((one) + (two) < (one)) + (((one) + (two) < (one)) ? (giterr_set_oom(), 1) : 0) /** Check for integer overflow from multiplication */ #define GIT_ALLOC_OVERFLOW_MULTIPLY(one, two) \ - (one && ((one) * (two)) / (one) != (two)) + ((one && ((one) * (two)) / (one) != (two)) ? (giterr_set_oom(), 1) : 0) /** Check for additive overflow, failing if it would occur. */ #define GITERR_CHECK_ALLOC_ADD(one, two) \ - if (GIT_ALLOC_OVERFLOW_ADD(one, two)) { \ - giterr_set_oom(); \ - return -1; \ - } + if (GIT_ALLOC_OVERFLOW_ADD(one, two)) { return -1; } /** Check for multiplicative overflow, failing if it would occur. */ #define GITERR_CHECK_ALLOC_MULTIPLY(nelem, elsize) \ - if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize)) { \ - giterr_set_oom(); \ - return -1; \ - } + if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize)) { return -1; } /* NOTE: other giterr functions are in the public errors.h header file */ diff --git a/src/config_file.c b/src/config_file.c index 36f78563b..39e9ff841 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -905,7 +905,6 @@ static char *reader_readline(struct reader *reader, bool skip_whitespace) if (GIT_ALLOC_OVERFLOW_ADD(line_len, 1) || (line = git__malloc(line_len + 1)) == NULL) { - giterr_set_oom(); return NULL; } @@ -1620,7 +1619,6 @@ static char *fixup_line(const char *ptr, int quote_count) if (GIT_ALLOC_OVERFLOW_ADD(ptr_len, 1) || (str = git__malloc(ptr_len + 1)) == NULL) { - giterr_set_oom(); return NULL; } diff --git a/src/delta.c b/src/delta.c index f704fdfb1..242f3abe3 100644 --- a/src/delta.c +++ b/src/delta.c @@ -138,7 +138,7 @@ static int lookup_index_alloc( index_len += hash_len; if (!git__is_ulong(index_len)) { - giterr_set_oom(); + giterr_set(GITERR_NOMEMORY, "Overly large delta"); return -1; } diff --git a/src/index.c b/src/index.c index 70090d12c..35f512ca4 100644 --- a/src/index.c +++ b/src/index.c @@ -833,10 +833,8 @@ static git_index_reuc_entry *reuc_entry_alloc(const char *path) struct reuc_entry_internal *entry; if (GIT_ALLOC_OVERFLOW_ADD(structlen, pathlen) || - GIT_ALLOC_OVERFLOW_ADD(structlen + pathlen, 1)) { - giterr_set_oom(); + GIT_ALLOC_OVERFLOW_ADD(structlen + pathlen, 1)) return NULL; - } entry = git__calloc(1, structlen + pathlen + 1); if (!entry) diff --git a/src/pack-objects.c b/src/pack-objects.c index cd0deb29a..288077078 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -504,10 +504,8 @@ static git_pobject **compute_write_order(git_packbuilder *pb) unsigned int i, wo_end, last_untagged; git_pobject **wo; - if ((wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL) { - giterr_set_oom(); + if ((wo = git__mallocarray(pb->nr_objects, sizeof(*wo))) == NULL) return NULL; - } for (i = 0; i < pb->nr_objects; i++) { git_pobject *po = pb->object_list + i; diff --git a/src/pool.c b/src/pool.c index 1599fc7a2..919e13d08 100644 --- a/src/pool.c +++ b/src/pool.c @@ -117,10 +117,8 @@ static void *pool_alloc_page(git_pool *pool, uint32_t size) } if (GIT_ALLOC_OVERFLOW_ADD(alloc_size, sizeof(git_pool_page)) || - !(page = git__calloc(1, alloc_size + sizeof(git_pool_page)))) { - giterr_set_oom(); + !(page = git__calloc(1, alloc_size + sizeof(git_pool_page)))) return NULL; - } page->size = alloc_size; page->avail = alloc_size - size; diff --git a/src/refs.c b/src/refs.c index 33e931db5..af3190ef9 100644 --- a/src/refs.c +++ b/src/refs.c @@ -41,10 +41,8 @@ static git_reference *alloc_ref(const char *name) if (GIT_ALLOC_OVERFLOW_ADD(reflen, namelen) || GIT_ALLOC_OVERFLOW_ADD(reflen + namelen, 1) || - (ref = git__calloc(1, reflen + namelen + 1)) == NULL) { - giterr_set_oom(); + (ref = git__calloc(1, reflen + namelen + 1)) == NULL) return NULL; - } memcpy(ref->name, name, namelen + 1); diff --git a/src/tree.c b/src/tree.c index 2c8b89291..573e56447 100644 --- a/src/tree.c +++ b/src/tree.c @@ -89,10 +89,8 @@ static git_tree_entry *alloc_entry(const char *filename) if (GIT_ALLOC_OVERFLOW_ADD(tree_len, filename_len) || GIT_ALLOC_OVERFLOW_ADD(tree_len + filename_len, 1) || - !(entry = git__malloc(tree_len + filename_len + 1))) { - giterr_set_oom(); + !(entry = git__malloc(tree_len + filename_len + 1))) return NULL; - } memset(entry, 0x0, sizeof(git_tree_entry)); memcpy(entry->filename, filename, filename_len); diff --git a/src/util.h b/src/util.h index 7693ef809..40e06976d 100644 --- a/src/util.h +++ b/src/util.h @@ -64,10 +64,8 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n) length = p_strnlen(str, n); - if (GIT_ALLOC_OVERFLOW_ADD(length, 1)) { - giterr_set_oom(); + if (GIT_ALLOC_OVERFLOW_ADD(length, 1)) return NULL; - } ptr = git__malloc(length + 1); @@ -87,10 +85,8 @@ GIT_INLINE(char *) git__substrdup(const char *start, size_t n) { char *ptr; - if (GIT_ALLOC_OVERFLOW_ADD(n, 1) || !(ptr = git__malloc(n+1))) { - giterr_set_oom(); + if (GIT_ALLOC_OVERFLOW_ADD(n, 1) || !(ptr = git__malloc(n+1))) return NULL; - } memcpy(ptr, start, n); ptr[n] = '\0'; @@ -111,11 +107,8 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size) */ GIT_INLINE(void *) git__reallocarray(void *ptr, size_t nelem, size_t elsize) { - void *new_ptr = NULL; - if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize) || - !(new_ptr = realloc(ptr, nelem * elsize))) - giterr_set_oom(); - return new_ptr; + return GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize) ? + NULL : realloc(ptr, nelem * elsize); } /** diff --git a/src/win32/dir.c b/src/win32/dir.c index 9953289f6..7f2a5a56e 100644 --- a/src/win32/dir.c +++ b/src/win32/dir.c @@ -20,10 +20,8 @@ git__DIR *git__opendir(const char *dir) if (GIT_ALLOC_OVERFLOW_ADD(sizeof(*new), dirlen) || GIT_ALLOC_OVERFLOW_ADD(sizeof(*new) + dirlen, 1) || - !(new = git__calloc(1, sizeof(*new) + dirlen + 1))) { - giterr_set_oom(); + !(new = git__calloc(1, sizeof(*new) + dirlen + 1))) return NULL; - } memcpy(new->dir, dir, dirlen); diff --git a/tests/core/errors.c b/tests/core/errors.c index b6364100b..11e0bb2bb 100644 --- a/tests/core/errors.c +++ b/tests/core/errors.c @@ -149,3 +149,24 @@ void test_core_errors__integer_overflow_alloc_add(void) cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass); cl_assert_equal_s("Out of memory", giterr_last()->message); } + +void test_core_errors__integer_overflow_sets_oom(void) +{ + giterr_clear(); + cl_assert(!GIT_ALLOC_OVERFLOW_ADD(SIZE_MAX-1, 1)); + cl_assert_equal_p(NULL, giterr_last()); + + giterr_clear(); + cl_assert(!GIT_ALLOC_OVERFLOW_ADD(42, 69)); + cl_assert_equal_p(NULL, giterr_last()); + + giterr_clear(); + cl_assert(GIT_ALLOC_OVERFLOW_ADD(SIZE_MAX, SIZE_MAX)); + cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass); + cl_assert_equal_s("Out of memory", giterr_last()->message); + + giterr_clear(); + cl_assert(GIT_ALLOC_OVERFLOW_MULTIPLY(SIZE_MAX, SIZE_MAX)); + cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass); + cl_assert_equal_s("Out of memory", giterr_last()->message); +} From ec3b4d35f636c26d3c9b5703c3b7f87683800af8 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 11 Feb 2015 11:20:05 -0500 Subject: [PATCH 06/15] Use `size_t` to hold size of arrays Use `size_t` to hold the size of arrays to ease overflow checking, lest we check for overflow of a `size_t` then promptly truncate by packing the length into a smaller type. --- src/array.h | 4 ++-- src/filebuf.c | 13 +++++++------ src/pack-objects.c | 10 +++++++++- src/transports/smart_pkt.c | 17 ++++++++++++----- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/array.h b/src/array.h index 0d0795370..7c4dbdbc1 100644 --- a/src/array.h +++ b/src/array.h @@ -23,7 +23,7 @@ * * typedef git_array_t(my_struct) my_struct_array_t; */ -#define git_array_t(type) struct { type *ptr; uint32_t size, asize; } +#define git_array_t(type) struct { type *ptr; size_t size, asize; } #define GIT_ARRAY_INIT { NULL, 0, 0 } @@ -45,7 +45,7 @@ typedef git_array_t(char) git_array_generic_t; GIT_INLINE(void *) git_array_grow(void *_a, size_t item_size) { volatile git_array_generic_t *a = _a; - uint32_t new_size; + size_t new_size; char *new_array; if (a->size < 8) { diff --git a/src/filebuf.c b/src/filebuf.c index 1a9157558..94f2bec32 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -408,8 +408,8 @@ int git_filebuf_reserve(git_filebuf *file, void **buffer, size_t len) int git_filebuf_printf(git_filebuf *file, const char *format, ...) { va_list arglist; - size_t space_left; - int len, res; + size_t space_left, len; + int written, res; char *tmp_buffer; ENSURE_BUF_OK(file); @@ -418,15 +418,16 @@ int git_filebuf_printf(git_filebuf *file, const char *format, ...) do { va_start(arglist, format); - len = p_vsnprintf((char *)file->buffer + file->buf_pos, space_left, format, arglist); + written = p_vsnprintf((char *)file->buffer + file->buf_pos, space_left, format, arglist); va_end(arglist); - if (len < 0) { + if (written < 0) { file->last_error = BUFERR_MEM; return -1; } - if ((size_t)len + 1 <= space_left) { + len = written; + if (len + 1 <= space_left) { file->buf_pos += len; return 0; } @@ -436,7 +437,7 @@ int git_filebuf_printf(git_filebuf *file, const char *format, ...) space_left = file->buf_size - file->buf_pos; - } while ((size_t)len + 1 <= space_left); + } while (len + 1 <= space_left); if (GIT_ALLOC_OVERFLOW_ADD(len, 1) || !(tmp_buffer = git__malloc(len + 1))) { diff --git a/src/pack-objects.c b/src/pack-objects.c index 288077078..9b56234b5 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -190,6 +190,7 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, { git_pobject *po; khiter_t pos; + size_t newsize; int ret; assert(pb && oid); @@ -203,7 +204,14 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, if (pb->nr_objects >= pb->nr_alloc) { GITERR_CHECK_ALLOC_ADD(pb->nr_alloc, 1024); GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc + 1024, 3 / 2); - pb->nr_alloc = (pb->nr_alloc + 1024) * 3 / 2; + newsize = (pb->nr_alloc + 1024) * 3 / 2; + + if (!git__is_uint32(newsize)) { + giterr_set(GITERR_NOMEMORY, "Packfile too large to fit in memory."); + return -1; + } + + pb->nr_alloc = (uint32_t)newsize; pb->object_list = git__reallocarray(pb->object_list, pb->nr_alloc, sizeof(*po)); diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index ad81ad1b0..11c6215ff 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -484,7 +484,7 @@ static int buffer_want_with_caps(const git_remote_head *head, transport_smart_ca { git_buf str = GIT_BUF_INIT; char oid[GIT_OID_HEXSZ +1] = {0}; - unsigned int len; + size_t len; /* Prefer multi_ack_detailed */ if (caps->multi_ack_detailed) @@ -510,12 +510,19 @@ static int buffer_want_with_caps(const git_remote_head *head, transport_smart_ca if (git_buf_oom(&str)) return -1; - len = (unsigned int) - (strlen("XXXXwant ") + GIT_OID_HEXSZ + 1 /* NUL */ + - git_buf_len(&str) + 1 /* LF */); + len = strlen("XXXXwant ") + GIT_OID_HEXSZ + 1 /* NUL */ + + git_buf_len(&str) + 1 /* LF */; + + if (len > 0xffff) { + giterr_set(GITERR_NET, + "Tried to produce packet with invalid length %d", len); + return -1; + } + git_buf_grow_by(buf, len); git_oid_fmt(oid, &head->oid); - git_buf_printf(buf, "%04xwant %s %s\n", len, oid, git_buf_cstr(&str)); + git_buf_printf(buf, + "%04xwant %s %s\n", (unsigned int)len, oid, git_buf_cstr(&str)); git_buf_free(&str); return git_buf_oom(buf); From 8d534b475829edf87c8126fc4ae30f593172f317 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 11 Feb 2015 13:01:00 -0500 Subject: [PATCH 07/15] p_read: ensure requested len is ssize_t Ensure that the given length to `p_read` is of ssize_t and ensure that callers test the return as if it were an `ssize_t`. --- src/fileops.c | 5 +++++ src/posix.c | 12 +++++++++++- src/util.h | 7 +++++++ 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/fileops.c b/src/fileops.c index eb24013e8..420ed70a2 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -127,6 +127,11 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) git_buf_clear(buf); + if (!git__is_ssizet(len)) { + giterr_set(GITERR_INVALID, "Read too large."); + return -1; + } + GITERR_CHECK_ALLOC_ADD(len, 1); if (git_buf_grow(buf, len + 1) < 0) return -1; diff --git a/src/posix.c b/src/posix.c index d5e6875b5..8d86aa8bf 100644 --- a/src/posix.c +++ b/src/posix.c @@ -155,6 +155,14 @@ ssize_t p_read(git_file fd, void *buf, size_t cnt) { char *b = buf; + if (!git__is_ssizet(cnt)) { +#ifdef GIT_WIN32 + SetLastError(ERROR_INVALID_PARAMETER); +#endif + errno = EINVAL; + return -1; + } + while (cnt) { ssize_t r; #ifdef GIT_WIN32 @@ -229,7 +237,9 @@ int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offs out->data = malloc(len); GITERR_CHECK_ALLOC(out->data); - if ((p_lseek(fd, offset, SEEK_SET) < 0) || ((size_t)p_read(fd, out->data, len) != len)) { + if (!git__is_ssizet(len) || + (p_lseek(fd, offset, SEEK_SET) < 0) || + (p_read(fd, out->data, len) != (ssize_t)len)) { giterr_set(GITERR_OS, "mmap emulation failed"); return -1; } diff --git a/src/util.h b/src/util.h index 40e06976d..dcdaf4363 100644 --- a/src/util.h +++ b/src/util.h @@ -153,6 +153,13 @@ GIT_INLINE(int) git__is_sizet(git_off_t p) return p == (git_off_t)r; } +/** @return true if p fits into the range of an ssize_t */ +GIT_INLINE(int) git__is_ssizet(size_t p) +{ + ssize_t r = (ssize_t)p; + return p == (size_t)r; +} + /** @return true if p fits into the range of a uint32_t */ GIT_INLINE(int) git__is_uint32(size_t p) { From 190b76a69875da49973d1a5d8fd3b96c0cd43425 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 11 Feb 2015 14:52:08 -0500 Subject: [PATCH 08/15] Introduce git__add_sizet_overflow and friends Add some helper functions to check for overflow in a type-specific manner. --- src/common.h | 6 ++-- src/integer.h | 77 ++++++++++++++++++++++++++++++++++++++++++++++ src/pack-objects.c | 4 +-- src/util.h | 28 ----------------- 4 files changed, 83 insertions(+), 32 deletions(-) create mode 100644 src/integer.h diff --git a/src/common.h b/src/common.h index 8b7d6936d..530e320e2 100644 --- a/src/common.h +++ b/src/common.h @@ -58,6 +58,7 @@ #include "git2/types.h" #include "git2/errors.h" #include "thread-utils.h" +#include "integer.h" #include @@ -174,13 +175,14 @@ GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int v GITERR_CHECK_VERSION(&(VERSION), _tmpl.version, #TYPE); \ memcpy((PTR), &_tmpl, sizeof(_tmpl)); } while (0) + /** Check for integer overflow from addition or multiplication */ #define GIT_ALLOC_OVERFLOW_ADD(one, two) \ - (((one) + (two) < (one)) ? (giterr_set_oom(), 1) : 0) + (!git__add_sizet_overflow(NULL, (one), (two)) ? (giterr_set_oom(), 1) : 0) /** Check for integer overflow from multiplication */ #define GIT_ALLOC_OVERFLOW_MULTIPLY(one, two) \ - ((one && ((one) * (two)) / (one) != (two)) ? (giterr_set_oom(), 1) : 0) + (!git__multiply_sizet_overflow(NULL, (one), (two)) ? (giterr_set_oom(), 1) : 0) /** Check for additive overflow, failing if it would occur. */ #define GITERR_CHECK_ALLOC_ADD(one, two) \ diff --git a/src/integer.h b/src/integer.h new file mode 100644 index 000000000..a9ed10aae --- /dev/null +++ b/src/integer.h @@ -0,0 +1,77 @@ +/* + * Copyright (C) the libgit2 contributors. All rights reserved. + * + * This file is part of libgit2, distributed under the GNU GPL v2 with + * a Linking Exception. For full terms see the included COPYING file. + */ +#ifndef INCLUDE_integer_h__ +#define INCLUDE_integer_h__ + +/** @return true if p fits into the range of a size_t */ +GIT_INLINE(int) git__is_sizet(git_off_t p) +{ + size_t r = (size_t)p; + return p == (git_off_t)r; +} + +/** @return true if p fits into the range of an ssize_t */ +GIT_INLINE(int) git__is_ssizet(size_t p) +{ + ssize_t r = (ssize_t)p; + return p == (size_t)r; +} + +/** @return true if p fits into the range of a uint32_t */ +GIT_INLINE(int) git__is_uint32(size_t p) +{ + uint32_t r = (uint32_t)p; + return p == (size_t)r; +} + +/** @return true if p fits into the range of an unsigned long */ +GIT_INLINE(int) git__is_ulong(git_off_t p) +{ + unsigned long r = (unsigned long)p; + return p == (git_off_t)r; +} + +/** + * Sets `one + two` into `out`, unless the arithmetic would overflow. + * @return true if the result fits in a `uint64_t`, false on overflow. + */ +GIT_INLINE(bool) git__add_uint64_overflow(uint64_t *out, uint64_t one, uint64_t two) +{ + if (UINT64_MAX - one < two) + return false; + if (out) + *out = one + two; + return true; +} + +/** + * Sets `one + two` into `out`, unless the arithmetic would overflow. + * @return true if the result fits in a `size_t`, false on overflow. + */ +GIT_INLINE(bool) git__add_sizet_overflow(size_t *out, size_t one, size_t two) +{ + if (SIZE_MAX - one < two) + return false; + if (out) + *out = one + two; + return true; +} + +/** + * Sets `one * two` into `out`, unless the arithmetic would overflow. + * @return true if the result fits in a `size_t`, false on overflow. + */ +GIT_INLINE(bool) git__multiply_sizet_overflow(size_t *out, size_t one, size_t two) +{ + if (one && SIZE_MAX / one < two) + return false; + if (out) + *out = one * two; + return true; +} + +#endif /* INCLUDE_integer_h__ */ diff --git a/src/pack-objects.c b/src/pack-objects.c index 9b56234b5..8236ef9f3 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -833,8 +833,8 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg, trg_object->delta_data = NULL; } if (delta_cacheable(pb, src_size, trg_size, delta_size)) { - GITERR_CHECK_ALLOC_ADD(pb->delta_cache_size, delta_size); - pb->delta_cache_size += delta_size; + if (!git__add_uint64_overflow(&pb->delta_cache_size, pb->delta_cache_size, delta_size)) + return -1; git_packbuilder__cache_unlock(pb); diff --git a/src/util.h b/src/util.h index dcdaf4363..86139ef77 100644 --- a/src/util.h +++ b/src/util.h @@ -146,34 +146,6 @@ extern int git__strtol64(int64_t *n, const char *buff, const char **end_buf, int extern void git__hexdump(const char *buffer, size_t n); extern uint32_t git__hash(const void *key, int len, uint32_t seed); -/** @return true if p fits into the range of a size_t */ -GIT_INLINE(int) git__is_sizet(git_off_t p) -{ - size_t r = (size_t)p; - return p == (git_off_t)r; -} - -/** @return true if p fits into the range of an ssize_t */ -GIT_INLINE(int) git__is_ssizet(size_t p) -{ - ssize_t r = (ssize_t)p; - return p == (size_t)r; -} - -/** @return true if p fits into the range of a uint32_t */ -GIT_INLINE(int) git__is_uint32(size_t p) -{ - uint32_t r = (uint32_t)p; - return p == (size_t)r; -} - -/** @return true if p fits into the range of an unsigned long */ -GIT_INLINE(int) git__is_ulong(git_off_t p) -{ - unsigned long r = (unsigned long)p; - return p == (git_off_t)r; -} - /* 32-bit cross-platform rotl */ #ifdef _MSC_VER /* use built-in method in MSVC */ # define git__rotl(v, s) (uint32_t)_rotl(v, s) From 1ad48c8a08c4bb3b10c0344d838494de9cee087e Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 11 Feb 2015 17:36:57 -0500 Subject: [PATCH 09/15] khash: update to klib f28c067 --- src/khash.h | 57 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/khash.h b/src/khash.h index 242204464..e5789c493 100644 --- a/src/khash.h +++ b/src/khash.h @@ -46,6 +46,19 @@ int main() { */ /* + 2013-05-02 (0.2.8): + + * Use quadratic probing. When the capacity is power of 2, stepping function + i*(i+1)/2 guarantees to traverse each bucket. It is better than double + hashing on cache performance and is more robust than linear probing. + + In theory, double hashing should be more robust than quadratic probing. + However, my implementation is probably not for large hash tables, because + the second hash function is closely tied to the first hash function, + which reduce the effectiveness of double hashing. + + Reference: http://research.cs.vt.edu/AVresearch/hashing/quadratic.php + 2011-12-29 (0.2.7): * Minor code clean up; no actual effect. @@ -110,13 +123,13 @@ int main() { Generic hash table library. */ -#define AC_VERSION_KHASH_H "0.2.6" +#define AC_VERSION_KHASH_H "0.2.8" #include #include #include -/* compipler specific configuration */ +/* compiler specific configuration */ #if UINT_MAX == 0xffffffffu typedef unsigned int khint32_t; @@ -130,11 +143,13 @@ typedef unsigned long khint64_t; typedef unsigned long long khint64_t; #endif +#ifndef kh_inline #ifdef _MSC_VER #define kh_inline __inline #else #define kh_inline inline #endif +#endif /* kh_inline */ typedef khint32_t khint_t; typedef khint_t khiter_t; @@ -147,12 +162,6 @@ typedef khint_t khiter_t; #define __ac_set_isboth_false(flag, i) (flag[i>>4]&=~(3ul<<((i&0xfU)<<1))) #define __ac_set_isdel_true(flag, i) (flag[i>>4]|=1ul<<((i&0xfU)<<1)) -#ifdef KHASH_LINEAR -#define __ac_inc(k, m) 1 -#else -#define __ac_inc(k, m) (((k)>>3 ^ (k)<<3) | 1) & (m) -#endif - #define __ac_fsize(m) ((m) < 16? 1 : (m)>>4) #ifndef kroundup32 @@ -175,7 +184,7 @@ typedef khint_t khiter_t; static const double __ac_HASH_UPPER = 0.77; #define __KHASH_TYPE(name, khkey_t, khval_t) \ - typedef struct { \ + typedef struct kh_##name##_s { \ khint_t n_buckets, size, n_occupied, upper_bound; \ khint32_t *flags; \ khkey_t *keys; \ @@ -213,19 +222,19 @@ static const double __ac_HASH_UPPER = 0.77; SCOPE khint_t kh_get_##name(const kh_##name##_t *h, khkey_t key) \ { \ if (h->n_buckets) { \ - khint_t inc, k, i, last, mask; \ + khint_t k, i, last, mask, step = 0; \ mask = h->n_buckets - 1; \ k = __hash_func(key); i = k & mask; \ - inc = __ac_inc(k, mask); last = i; /* inc==1 for linear probing */ \ + last = i; \ while (!__ac_isempty(h->flags, i) && (__ac_isdel(h->flags, i) || !__hash_equal(h->keys[i], key))) { \ - i = (i + inc) & mask; \ + i = (i + (++step)) & mask; \ if (i == last) return h->n_buckets; \ } \ return __ac_iseither(h->flags, i)? h->n_buckets : i; \ } else return 0; \ } \ SCOPE int kh_resize_##name(kh_##name##_t *h, khint_t new_n_buckets) \ - { /* This function uses 0.25*n_bucktes bytes of working space instead of [sizeof(key_t+val_t)+.25]*n_buckets. */ \ + { /* This function uses 0.25*n_buckets bytes of working space instead of [sizeof(key_t+val_t)+.25]*n_buckets. */ \ khint32_t *new_flags = 0; \ khint_t j = 1; \ { \ @@ -238,11 +247,11 @@ static const double __ac_HASH_UPPER = 0.77; memset(new_flags, 0xaa, __ac_fsize(new_n_buckets) * sizeof(khint32_t)); \ if (h->n_buckets < new_n_buckets) { /* expand */ \ khkey_t *new_keys = (khkey_t*)krealloc((void *)h->keys, new_n_buckets * sizeof(khkey_t)); \ - if (!new_keys) return -1; \ + if (!new_keys) { kfree(new_flags); return -1; } \ h->keys = new_keys; \ if (kh_is_map) { \ khval_t *new_vals = (khval_t*)krealloc((void *)h->vals, new_n_buckets * sizeof(khval_t)); \ - if (!new_vals) return -1; \ + if (!new_vals) { kfree(new_flags); return -1; } \ h->vals = new_vals; \ } \ } /* otherwise shrink */ \ @@ -258,11 +267,10 @@ static const double __ac_HASH_UPPER = 0.77; if (kh_is_map) val = h->vals[j]; \ __ac_set_isdel_true(h->flags, j); \ while (1) { /* kick-out process; sort of like in Cuckoo hashing */ \ - khint_t inc, k, i; \ + khint_t k, i, step = 0; \ k = __hash_func(key); \ i = k & new_mask; \ - inc = __ac_inc(k, new_mask); \ - while (!__ac_isempty(new_flags, i)) i = (i + inc) & new_mask; \ + while (!__ac_isempty(new_flags, i)) i = (i + (++step)) & new_mask; \ __ac_set_isempty_false(new_flags, i); \ if (i < h->n_buckets && __ac_iseither(h->flags, i) == 0) { /* kick out the existing element */ \ { khkey_t tmp = h->keys[i]; h->keys[i] = key; key = tmp; } \ @@ -301,14 +309,14 @@ static const double __ac_HASH_UPPER = 0.77; } \ } /* TODO: to implement automatically shrinking; resize() already support shrinking */ \ { \ - khint_t inc, k, i, site, last, mask = h->n_buckets - 1; \ + khint_t k, i, site, last, mask = h->n_buckets - 1, step = 0; \ x = site = h->n_buckets; k = __hash_func(key); i = k & mask; \ if (__ac_isempty(h->flags, i)) x = i; /* for speed up */ \ else { \ - inc = __ac_inc(k, mask); last = i; \ + last = i; \ while (!__ac_isempty(h->flags, i) && (__ac_isdel(h->flags, i) || !__hash_equal(h->keys[i], key))) { \ if (__ac_isdel(h->flags, i)) site = i; \ - i = (i + inc) & mask; \ + i = (i + (++step)) & mask; \ if (i == last) { x = site; break; } \ } \ if (x == h->n_buckets) { \ @@ -449,7 +457,8 @@ static kh_inline khint_t __ac_Wang_hash(khint_t key) @param name Name of the hash table [symbol] @param h Pointer to the hash table [khash_t(name)*] @param k Key [type of keys] - @param r Extra return code: 0 if the key is present in the hash table; + @param r Extra return code: -1 if the operation failed; + 0 if the key is present in the hash table; 1 if the bucket is empty (never used); 2 if the element in the bucket has been deleted [int*] @return Iterator to the inserted element [khint_t] @@ -461,7 +470,7 @@ static kh_inline khint_t __ac_Wang_hash(khint_t key) @param name Name of the hash table [symbol] @param h Pointer to the hash table [khash_t(name)*] @param k Key [type of keys] - @return Iterator to the found element, or kh_end(h) is the element is absent [khint_t] + @return Iterator to the found element, or kh_end(h) if the element is absent [khint_t] */ #define kh_get(name, h, k) kh_get_##name(h, k) @@ -607,4 +616,4 @@ typedef const char *kh_cstr_t; #define KHASH_MAP_INIT_STR(name, khval_t) \ KHASH_INIT(name, kh_cstr_t, khval_t, 1, kh_str_hash_func, kh_str_hash_equal) -#endif /* __AC_KHASH_H */ +#endif /* __AC_KHASH_H */ \ No newline at end of file From 650e45f69124bd8b53ecefddeb214a82538ab2c1 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 11 Feb 2015 17:51:17 -0500 Subject: [PATCH 10/15] Update `khash.h` to pull request 42 https://github.com/attractivechaos/klib/pull/42/files introduces `kreallocarray`. Hook that up as our `git__reallocarray`. --- src/khash.h | 13 ++++++++----- src/offmap.h | 1 + src/oidmap.h | 1 + src/strmap.h | 1 + 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/khash.h b/src/khash.h index e5789c493..818ac833b 100644 --- a/src/khash.h +++ b/src/khash.h @@ -177,6 +177,9 @@ typedef khint_t khiter_t; #ifndef krealloc #define krealloc(P,Z) realloc(P,Z) #endif +#ifndef kreallocarray +#define kreallocarray(P,N,Z) ((SIZE_MAX - N < Z) ? NULL : krealloc(P, (N*Z))) +#endif #ifndef kfree #define kfree(P) free(P) #endif @@ -242,15 +245,15 @@ static const double __ac_HASH_UPPER = 0.77; if (new_n_buckets < 4) new_n_buckets = 4; \ if (h->size >= (khint_t)(new_n_buckets * __ac_HASH_UPPER + 0.5)) j = 0; /* requested size is too small */ \ else { /* hash table size to be changed (shrink or expand); rehash */ \ - new_flags = (khint32_t*)kmalloc(__ac_fsize(new_n_buckets) * sizeof(khint32_t)); \ + new_flags = (khint32_t*)kreallocarray(NULL, __ac_fsize(new_n_buckets), sizeof(khint32_t)); \ if (!new_flags) return -1; \ memset(new_flags, 0xaa, __ac_fsize(new_n_buckets) * sizeof(khint32_t)); \ if (h->n_buckets < new_n_buckets) { /* expand */ \ - khkey_t *new_keys = (khkey_t*)krealloc((void *)h->keys, new_n_buckets * sizeof(khkey_t)); \ + khkey_t *new_keys = (khkey_t*)kreallocarray((void *)h->keys, new_n_buckets, sizeof(khkey_t)); \ if (!new_keys) { kfree(new_flags); return -1; } \ h->keys = new_keys; \ if (kh_is_map) { \ - khval_t *new_vals = (khval_t*)krealloc((void *)h->vals, new_n_buckets * sizeof(khval_t)); \ + khval_t *new_vals = (khval_t*)kreallocarray((void *)h->vals, new_n_buckets, sizeof(khval_t)); \ if (!new_vals) { kfree(new_flags); return -1; } \ h->vals = new_vals; \ } \ @@ -285,8 +288,8 @@ static const double __ac_HASH_UPPER = 0.77; } \ } \ if (h->n_buckets > new_n_buckets) { /* shrink the hash table */ \ - h->keys = (khkey_t*)krealloc((void *)h->keys, new_n_buckets * sizeof(khkey_t)); \ - if (kh_is_map) h->vals = (khval_t*)krealloc((void *)h->vals, new_n_buckets * sizeof(khval_t)); \ + h->keys = (khkey_t*)kreallocarray((void *)h->keys, new_n_buckets, sizeof(khkey_t)); \ + if (kh_is_map) h->vals = (khval_t*)kreallocarray((void *)h->vals, new_n_buckets, sizeof(khval_t)); \ } \ kfree(h->flags); /* free the working space */ \ h->flags = new_flags; \ diff --git a/src/offmap.h b/src/offmap.h index cd46fd687..9471e7b91 100644 --- a/src/offmap.h +++ b/src/offmap.h @@ -13,6 +13,7 @@ #define kmalloc git__malloc #define kcalloc git__calloc #define krealloc git__realloc +#define kreallocarray git__reallocarray #define kfree git__free #include "khash.h" diff --git a/src/oidmap.h b/src/oidmap.h index b871a7926..5e3b44bfb 100644 --- a/src/oidmap.h +++ b/src/oidmap.h @@ -13,6 +13,7 @@ #define kmalloc git__malloc #define kcalloc git__calloc #define krealloc git__realloc +#define kreallocarray git__reallocarray #define kfree git__free #include "khash.h" diff --git a/src/strmap.h b/src/strmap.h index 8985aaf7e..dfbe5639f 100644 --- a/src/strmap.h +++ b/src/strmap.h @@ -12,6 +12,7 @@ #define kmalloc git__malloc #define kcalloc git__calloc #define krealloc git__realloc +#define kreallocarray git__reallocarray #define kfree git__free #include "khash.h" From f1453c59b2afb9dab43281bfe9f1ba34cf6e0d02 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 12 Feb 2015 12:19:37 -0500 Subject: [PATCH 11/15] Make our overflow check look more like gcc/clang's Make our overflow checking look more like gcc and clang's, so that we can substitute it out with the compiler instrinsics on platforms that support it. This means dropping the ability to pass `NULL` as an out parameter. As a result, the macros also get updated to reflect this as well. --- src/array.h | 5 +- src/blame_git.c | 10 +-- src/buf_text.c | 22 +++--- src/buffer.c | 143 ++++++++++++++++++++----------------- src/common.h | 20 +++--- src/config_file.c | 24 +++---- src/delta-apply.c | 6 +- src/delta.c | 17 ++--- src/diff.c | 5 +- src/diff_driver.c | 9 +-- src/diff_patch.c | 13 ++-- src/diff_tform.c | 5 +- src/filebuf.c | 12 ++-- src/fileops.c | 18 +++-- src/filter.c | 19 ++--- src/index.c | 17 ++--- src/integer.h | 28 ++++---- src/iterator.c | 7 +- src/merge.c | 9 ++- src/odb.c | 15 ++-- src/odb_loose.c | 42 +++++------ src/odb_mempack.c | 5 +- src/pack-objects.c | 7 +- src/pack.c | 21 +++--- src/path.c | 43 +++++------ src/pool.c | 15 ++-- src/refs.c | 22 +++--- src/sortedcache.c | 8 +-- src/tag.c | 10 +-- src/transports/cred.c | 8 +-- src/transports/smart_pkt.c | 50 +++++++------ src/tree.c | 18 ++--- src/util.h | 19 +++-- src/vector.c | 3 +- src/win32/dir.c | 8 +-- tests/core/errors.c | 16 +++-- 36 files changed, 352 insertions(+), 347 deletions(-) diff --git a/src/array.h b/src/array.h index 7c4dbdbc1..7cd9b7153 100644 --- a/src/array.h +++ b/src/array.h @@ -51,10 +51,9 @@ GIT_INLINE(void *) git_array_grow(void *_a, size_t item_size) if (a->size < 8) { new_size = 8; } else { - if (GIT_ALLOC_OVERFLOW_MULTIPLY(a->size, 3 / 2)) + if (GIT_MULTIPLY_SIZET_OVERFLOW(&new_size, a->size, 3)) goto on_oom; - - new_size = a->size * 3 / 2; + new_size /= 2; } if ((new_array = git__reallocarray(a->ptr, new_size, item_size)) == NULL) diff --git a/src/blame_git.c b/src/blame_git.c index 05aef5d99..e863efe2e 100644 --- a/src/blame_git.c +++ b/src/blame_git.c @@ -36,14 +36,14 @@ static void origin_decref(git_blame__origin *o) static int make_origin(git_blame__origin **out, git_commit *commit, const char *path) { git_blame__origin *o; - size_t path_len = strlen(path); + size_t path_len = strlen(path), alloc_len; int error = 0; - GITERR_CHECK_ALLOC_ADD(sizeof(*o), path_len); - GITERR_CHECK_ALLOC_ADD(sizeof(*o) + path_len, 1); - - o = git__calloc(1, sizeof(*o) + path_len + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(*o), path_len); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1); + o = git__calloc(1, alloc_len); GITERR_CHECK_ALLOC(o); + o->commit = commit; o->refcnt = 1; strcpy(o->path, path); diff --git a/src/buf_text.c b/src/buf_text.c index 08b86f4cc..864e39cab 100644 --- a/src/buf_text.c +++ b/src/buf_text.c @@ -13,7 +13,7 @@ int git_buf_text_puts_escaped( const char *esc_with) { const char *scan; - size_t total = 0, esc_len = strlen(esc_with), count; + size_t total = 0, esc_len = strlen(esc_with), count, alloclen; if (!string) return 0; @@ -29,8 +29,8 @@ int git_buf_text_puts_escaped( scan += count; } - GITERR_CHECK_ALLOC_ADD(total, 1); - if (git_buf_grow_by(buf, total + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&alloclen, total, 1); + if (git_buf_grow_by(buf, alloclen) < 0) return -1; for (scan = string; *scan; ) { @@ -66,6 +66,7 @@ int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src) const char *scan = src->ptr; const char *scan_end = src->ptr + src->size; const char *next = memchr(scan, '\r', src->size); + size_t new_size; char *out; assert(tgt != src); @@ -74,8 +75,8 @@ int git_buf_text_crlf_to_lf(git_buf *tgt, const git_buf *src) return git_buf_set(tgt, src->ptr, src->size); /* reduce reallocs while in the loop */ - GITERR_CHECK_ALLOC_ADD(src->size, 1); - if (git_buf_grow(tgt, src->size + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&new_size, src->size, 1); + if (git_buf_grow(tgt, new_size) < 0) return -1; out = tgt->ptr; @@ -113,6 +114,7 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) const char *end = start + src->size; const char *scan = start; const char *next = memchr(scan, '\n', src->size); + size_t alloclen; assert(tgt != src); @@ -120,9 +122,9 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) return git_buf_set(tgt, src->ptr, src->size); /* attempt to reduce reallocs while in the loop */ - GITERR_CHECK_ALLOC_ADD(src->size, src->size >> 4); - GITERR_CHECK_ALLOC_ADD(src->size + (src->size >> 4), 1); - if (git_buf_grow(tgt, src->size + (src->size >> 4) + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&alloclen, src->size, src->size >> 4); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); + if (git_buf_grow(tgt, alloclen) < 0) return -1; tgt->size = 0; @@ -135,8 +137,8 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) return GIT_PASSTHROUGH; } - GITERR_CHECK_ALLOC_ADD(copylen, 3); - if (git_buf_grow_by(tgt, copylen + 3) < 0) + GITERR_CHECK_ALLOC_ADD(&alloclen, copylen, 3); + if (git_buf_grow_by(tgt, alloclen) < 0) return -1; if (next > scan) { diff --git a/src/buffer.c b/src/buffer.c index 0d0314439..3deb0329c 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -103,13 +103,14 @@ int git_buf_grow(git_buf *buffer, size_t target_size) int git_buf_grow_by(git_buf *buffer, size_t additional_size) { - if (GIT_ALLOC_OVERFLOW_ADD(buffer->size, additional_size)) { + size_t newsize; + + if (GIT_ADD_SIZET_OVERFLOW(&newsize, buffer->size, additional_size)) { buffer->ptr = git_buf__oom; return -1; } - return git_buf_try_grow( - buffer, buffer->size + additional_size, true, true); + return git_buf_try_grow(buffer, newsize, true, true); } void git_buf_free(git_buf *buf) @@ -146,12 +147,14 @@ void git_buf_clear(git_buf *buf) int git_buf_set(git_buf *buf, const void *data, size_t len) { + size_t alloclen; + if (len == 0 || data == NULL) { git_buf_clear(buf); } else { if (data != buf->ptr) { - GITERR_CHECK_ALLOC_ADD(len, 1); - ENSURE_SIZE(buf, len + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, len, 1); + ENSURE_SIZE(buf, alloclen); memmove(buf->ptr, data, len); } @@ -180,8 +183,9 @@ int git_buf_sets(git_buf *buf, const char *string) int git_buf_putc(git_buf *buf, char c) { - GITERR_CHECK_ALLOC_ADD(buf->size, 2); - ENSURE_SIZE(buf, buf->size + 2); + size_t new_size; + GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, 2); + ENSURE_SIZE(buf, new_size); buf->ptr[buf->size++] = c; buf->ptr[buf->size] = '\0'; return 0; @@ -189,9 +193,10 @@ int git_buf_putc(git_buf *buf, char c) int git_buf_putcn(git_buf *buf, char c, size_t len) { - GITERR_CHECK_ALLOC_ADD(buf->size, len); - GITERR_CHECK_ALLOC_ADD(buf->size + len, 1); - ENSURE_SIZE(buf, buf->size + len + 1); + size_t new_size; + GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, len); + GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1); + ENSURE_SIZE(buf, new_size); memset(buf->ptr + buf->size, c, len); buf->size += len; buf->ptr[buf->size] = '\0'; @@ -201,10 +206,13 @@ int git_buf_putcn(git_buf *buf, char c, size_t len) int git_buf_put(git_buf *buf, const char *data, size_t len) { if (len) { + size_t new_size; + assert(data); - GITERR_CHECK_ALLOC_ADD(buf->size, len); - GITERR_CHECK_ALLOC_ADD(buf->size + len, 1); - ENSURE_SIZE(buf, buf->size + len + 1); + + GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, len); + GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1); + ENSURE_SIZE(buf, new_size); memmove(buf->ptr + buf->size, data, len); buf->size += len; buf->ptr[buf->size] = '\0'; @@ -226,12 +234,13 @@ int git_buf_encode_base64(git_buf *buf, const char *data, size_t len) size_t extra = len % 3; uint8_t *write, a, b, c; const uint8_t *read = (const uint8_t *)data; - size_t blocks = (len / 3) + !!extra; + size_t blocks = (len / 3) + !!extra, alloclen; - GITERR_CHECK_ALLOC_MULTIPLY(blocks, 4); - GITERR_CHECK_ALLOC_ADD(buf->size, 4 * blocks); - GITERR_CHECK_ALLOC_ADD(buf->size + 4 * blocks, 1); - ENSURE_SIZE(buf, buf->size + 4 * blocks + 1); + GITERR_CHECK_ALLOC_ADD(&blocks, blocks, 1); + GITERR_CHECK_ALLOC_MULTIPLY(&alloclen, blocks, 4); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, buf->size); + + ENSURE_SIZE(buf, alloclen); write = (uint8_t *)&buf->ptr[buf->size]; /* convert each run of 3 bytes into 4 output bytes */ @@ -282,12 +291,12 @@ int git_buf_decode_base64(git_buf *buf, const char *base64, size_t len) { size_t i; int8_t a, b, c, d; - size_t orig_size = buf->size; + size_t orig_size = buf->size, new_size; assert(len % 4 == 0); - GITERR_CHECK_ALLOC_ADD(buf->size, len / 4 * 3); - GITERR_CHECK_ALLOC_ADD(buf->size + (len / 4 * 3), 1); - ENSURE_SIZE(buf, buf->size + (len / 4 * 3) + 1); + GITERR_CHECK_ALLOC_ADD(&new_size, (len / 4 * 3), buf->size); + GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1); + ENSURE_SIZE(buf, new_size); for (i = 0; i < len; i += 4) { if ((a = BASE64_DECODE_VALUE(base64[i])) < 0 || @@ -315,12 +324,13 @@ static const char b85str[] = int git_buf_encode_base85(git_buf *buf, const char *data, size_t len) { - size_t blocks = (len / 4) + !!(len % 4); + size_t blocks = (len / 4) + !!(len % 4), alloclen; - GITERR_CHECK_ALLOC_MULTIPLY(blocks, 5); - GITERR_CHECK_ALLOC_ADD(buf->size, 5 * blocks); - GITERR_CHECK_ALLOC_ADD(buf->size + 5 * blocks, 1); - ENSURE_SIZE(buf, buf->size + blocks * 5 + 1); + GITERR_CHECK_ALLOC_MULTIPLY(&alloclen, blocks, 5); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, buf->size); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); + + ENSURE_SIZE(buf, alloclen); while (len) { uint32_t acc = 0; @@ -353,15 +363,11 @@ int git_buf_encode_base85(git_buf *buf, const char *data, size_t len) int git_buf_vprintf(git_buf *buf, const char *format, va_list ap) { - size_t expected_size = strlen(format); + size_t expected_size, new_size; int len; - GITERR_CHECK_ALLOC_MULTIPLY(expected_size, 2); - expected_size *= 2; - - GITERR_CHECK_ALLOC_ADD(expected_size, buf->size); - expected_size += buf->size; - + GITERR_CHECK_ALLOC_MULTIPLY(&expected_size, strlen(format), 2); + GITERR_CHECK_ALLOC_ADD(&expected_size, expected_size, buf->size); ENSURE_SIZE(buf, expected_size); while (1) { @@ -387,9 +393,9 @@ int git_buf_vprintf(git_buf *buf, const char *format, va_list ap) break; } - GITERR_CHECK_ALLOC_ADD(buf->size, len); - GITERR_CHECK_ALLOC_ADD(buf->size + len, 1); - ENSURE_SIZE(buf, buf->size + len + 1); + GITERR_CHECK_ALLOC_ADD(&new_size, buf->size, len); + GITERR_CHECK_ALLOC_ADD(&new_size, new_size, 1); + ENSURE_SIZE(buf, new_size); } return 0; @@ -516,9 +522,11 @@ int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...) continue; segment_len = strlen(segment); - total_size += segment_len; + + GITERR_CHECK_ALLOC_ADD(&total_size, total_size, segment_len); + if (segment_len == 0 || segment[segment_len - 1] != separator) - ++total_size; /* space for separator */ + GITERR_CHECK_ALLOC_ADD(&total_size, total_size, 1); } va_end(ap); @@ -526,8 +534,8 @@ int git_buf_join_n(git_buf *buf, char separator, int nbuf, ...) if (total_size == 0) return 0; - GITERR_CHECK_ALLOC_ADD(total_size, 1); - if (git_buf_grow_by(buf, total_size + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&total_size, total_size, 1); + if (git_buf_grow_by(buf, total_size) < 0) return -1; out = buf->ptr + buf->size; @@ -588,6 +596,7 @@ int git_buf_join( { size_t strlen_a = str_a ? strlen(str_a) : 0; size_t strlen_b = strlen(str_b); + size_t alloc_len; int need_sep = 0; ssize_t offset_a = -1; @@ -605,10 +614,10 @@ int git_buf_join( if (str_a >= buf->ptr && str_a < buf->ptr + buf->size) offset_a = str_a - buf->ptr; - GITERR_CHECK_ALLOC_ADD(strlen_a, strlen_b); - GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b, need_sep); - GITERR_CHECK_ALLOC_ADD(strlen_a + strlen_b + need_sep, 1); - if (git_buf_grow(buf, strlen_a + strlen_b + need_sep + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&alloc_len, strlen_a, strlen_b); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, need_sep); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1); + if (git_buf_grow(buf, alloc_len) < 0) return -1; assert(buf->ptr); @@ -636,7 +645,10 @@ int git_buf_join3( const char *str_b, const char *str_c) { - size_t len_a = strlen(str_a), len_b = strlen(str_b), len_c = strlen(str_c); + size_t len_a = strlen(str_a), + len_b = strlen(str_b), + len_c = strlen(str_c), + len_total; int sep_a = 0, sep_b = 0; char *tgt; @@ -656,12 +668,12 @@ int git_buf_join3( sep_b = (str_b[len_b - 1] != separator); } - GITERR_CHECK_ALLOC_ADD(len_a, sep_a); - GITERR_CHECK_ALLOC_ADD(len_a + sep_a, len_b); - GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b, sep_b); - GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b, len_c); - GITERR_CHECK_ALLOC_ADD(len_a + sep_a + len_b + sep_b + len_c, 1); - if (git_buf_grow(buf, len_a + sep_a + len_b + sep_b + len_c + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&len_total, len_a, sep_a); + GITERR_CHECK_ALLOC_ADD(&len_total, len_total, len_b); + GITERR_CHECK_ALLOC_ADD(&len_total, len_total, sep_b); + GITERR_CHECK_ALLOC_ADD(&len_total, len_total, len_c); + GITERR_CHECK_ALLOC_ADD(&len_total, len_total, 1); + if (git_buf_grow(buf, len_total) < 0) return -1; tgt = buf->ptr; @@ -714,28 +726,25 @@ int git_buf_splice( const char *data, size_t nb_to_insert) { - size_t new_size; + char *splice_loc; + size_t new_size, alloc_size; - assert(buf && - where <= git_buf_len(buf) && - where + nb_to_remove <= git_buf_len(buf)); + assert(buf && where <= buf->size && nb_to_remove <= buf->size - where); + + splice_loc = buf->ptr + where; /* Ported from git.git * https://github.com/git/git/blob/16eed7c/strbuf.c#L159-176 */ - new_size = buf->size - nb_to_remove; + GITERR_CHECK_ALLOC_ADD(&new_size, (buf->size - nb_to_remove), nb_to_insert); + GITERR_CHECK_ALLOC_ADD(&alloc_size, new_size, 1); + ENSURE_SIZE(buf, alloc_size); - GITERR_CHECK_ALLOC_ADD(new_size, nb_to_insert); - new_size += nb_to_insert; + memmove(splice_loc + nb_to_insert, + splice_loc + nb_to_remove, + buf->size - where - nb_to_remove); - GITERR_CHECK_ALLOC_ADD(new_size, 1); - ENSURE_SIZE(buf, new_size + 1); - - memmove(buf->ptr + where + nb_to_insert, - buf->ptr + where + nb_to_remove, - buf->size - where - nb_to_remove); - - memcpy(buf->ptr + where, data, nb_to_insert); + memcpy(splice_loc, data, nb_to_insert); buf->size = new_size; buf->ptr[buf->size] = '\0'; diff --git a/src/common.h b/src/common.h index 530e320e2..8d1e89064 100644 --- a/src/common.h +++ b/src/common.h @@ -176,21 +176,21 @@ GIT_INLINE(void) git__init_structure(void *structure, size_t len, unsigned int v memcpy((PTR), &_tmpl, sizeof(_tmpl)); } while (0) -/** Check for integer overflow from addition or multiplication */ -#define GIT_ALLOC_OVERFLOW_ADD(one, two) \ - (!git__add_sizet_overflow(NULL, (one), (two)) ? (giterr_set_oom(), 1) : 0) +/** Check for additive overflow, setting an error if would occur. */ +#define GIT_ADD_SIZET_OVERFLOW(out, one, two) \ + (git__add_sizet_overflow(out, one, two) ? (giterr_set_oom(), 1) : 0) -/** Check for integer overflow from multiplication */ -#define GIT_ALLOC_OVERFLOW_MULTIPLY(one, two) \ - (!git__multiply_sizet_overflow(NULL, (one), (two)) ? (giterr_set_oom(), 1) : 0) +/** Check for additive overflow, setting an error if would occur. */ +#define GIT_MULTIPLY_SIZET_OVERFLOW(out, nelem, elsize) \ + (git__multiply_sizet_overflow(out, nelem, elsize) ? (giterr_set_oom(), 1) : 0) /** Check for additive overflow, failing if it would occur. */ -#define GITERR_CHECK_ALLOC_ADD(one, two) \ - if (GIT_ALLOC_OVERFLOW_ADD(one, two)) { return -1; } +#define GITERR_CHECK_ALLOC_ADD(out, one, two) \ + if (GIT_ADD_SIZET_OVERFLOW(out, one, two)) { return -1; } /** Check for multiplicative overflow, failing if it would occur. */ -#define GITERR_CHECK_ALLOC_MULTIPLY(nelem, elsize) \ - if (GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize)) { return -1; } +#define GITERR_CHECK_ALLOC_MULTIPLY(out, nelem, elsize) \ + if (GIT_MULTIPLY_SIZET_OVERFLOW(out, nelem, elsize)) { return -1; } /* NOTE: other giterr functions are in the public errors.h header file */ diff --git a/src/config_file.c b/src/config_file.c index 39e9ff841..8ccbe64cc 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -885,7 +885,7 @@ static char *reader_readline(struct reader *reader, bool skip_whitespace) { char *line = NULL; char *line_src, *line_end; - size_t line_len; + size_t line_len, alloc_len; line_src = reader->read_ptr; @@ -903,8 +903,8 @@ static char *reader_readline(struct reader *reader, bool skip_whitespace) line_len = line_end - line_src; - if (GIT_ALLOC_OVERFLOW_ADD(line_len, 1) || - (line = git__malloc(line_len + 1)) == NULL) { + if (GIT_ADD_SIZET_OVERFLOW(&alloc_len, line_len, 1) || + (line = git__malloc(alloc_len)) == NULL) { return NULL; } @@ -959,7 +959,7 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con int c, rpos; char *first_quote, *last_quote; git_buf buf = GIT_BUF_INIT; - size_t quoted_len, base_name_len = strlen(base_name); + size_t quoted_len, alloc_len, base_name_len = strlen(base_name); /* * base_name is what came before the space. We should be at the @@ -976,10 +976,10 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con return -1; } - GITERR_CHECK_ALLOC_ADD(base_name_len, quoted_len); - GITERR_CHECK_ALLOC_ADD(base_name_len + quoted_len, 2); + GITERR_CHECK_ALLOC_ADD(&alloc_len, base_name_len, quoted_len); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 2); - git_buf_grow(&buf, base_name_len + quoted_len + 2); + git_buf_grow(&buf, alloc_len); git_buf_printf(&buf, "%s.", base_name); rpos = 0; @@ -1050,9 +1050,7 @@ static int parse_section_header(struct reader *reader, char **section_out) return -1; } - line_len = (size_t)(name_end - line); - - GITERR_CHECK_ALLOC_ADD(line_len, 1); + GITERR_CHECK_ALLOC_ADD(&line_len, (size_t)(name_end - line), 1); name = git__malloc(line_len); GITERR_CHECK_ALLOC(name); @@ -1615,10 +1613,10 @@ static char *escape_value(const char *ptr) static char *fixup_line(const char *ptr, int quote_count) { char *str, *out, *esc; - size_t ptr_len = strlen(ptr); + size_t ptr_len = strlen(ptr), alloc_len; - if (GIT_ALLOC_OVERFLOW_ADD(ptr_len, 1) || - (str = git__malloc(ptr_len + 1)) == NULL) { + if (GIT_ADD_SIZET_OVERFLOW(&alloc_len, ptr_len, 1) || + (str = git__malloc(alloc_len)) == NULL) { return NULL; } diff --git a/src/delta-apply.c b/src/delta-apply.c index e46c9631c..89745faa0 100644 --- a/src/delta-apply.c +++ b/src/delta-apply.c @@ -57,7 +57,7 @@ int git__delta_apply( size_t delta_len) { const unsigned char *delta_end = delta + delta_len; - size_t base_sz, res_sz; + size_t base_sz, res_sz, alloc_sz; unsigned char *res_dp; /* Check that the base size matches the data we were given; @@ -74,8 +74,8 @@ int git__delta_apply( return -1; } - GITERR_CHECK_ALLOC_ADD(res_sz, 1); - res_dp = git__malloc(res_sz + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_sz, res_sz, 1); + res_dp = git__malloc(alloc_sz); GITERR_CHECK_ALLOC(res_dp); res_dp[res_sz] = '\0'; diff --git a/src/delta.c b/src/delta.c index 242f3abe3..d72d820d8 100644 --- a/src/delta.c +++ b/src/delta.c @@ -122,20 +122,13 @@ struct git_delta_index { static int lookup_index_alloc( void **out, unsigned long *out_len, size_t entries, size_t hash_count) { - size_t entries_len, hash_len, - index_len = sizeof(struct git_delta_index); + size_t entries_len, hash_len, index_len; - GITERR_CHECK_ALLOC_MULTIPLY(entries, sizeof(struct index_entry)); - entries_len = entries * sizeof(struct index_entry); + GITERR_CHECK_ALLOC_MULTIPLY(&entries_len, entries, sizeof(struct index_entry)); + GITERR_CHECK_ALLOC_MULTIPLY(&hash_len, hash_count, sizeof(struct index_entry *)); - GITERR_CHECK_ALLOC_ADD(index_len, entries_len); - index_len += entries_len; - - GITERR_CHECK_ALLOC_MULTIPLY(hash_count, sizeof(struct index_entry *)); - hash_len = hash_count * sizeof(struct index_entry *); - - GITERR_CHECK_ALLOC_ADD(index_len, hash_len); - index_len += hash_len; + GITERR_CHECK_ALLOC_ADD(&index_len, sizeof(struct git_delta_index), entries_len); + GITERR_CHECK_ALLOC_ADD(&index_len, index_len, hash_len); if (!git__is_ulong(index_len)) { giterr_set(GITERR_NOMEMORY, "Overly large delta"); diff --git a/src/diff.c b/src/diff.c index 75e9ae9a3..07eae03e7 100644 --- a/src/diff.c +++ b/src/diff.c @@ -1527,6 +1527,7 @@ int git_diff_format_email( char *summary = NULL, *loc = NULL; bool ignore_marker; unsigned int format_flags = 0; + size_t allocsize; int error; assert(out && diff && opts); @@ -1558,8 +1559,8 @@ int git_diff_format_email( goto on_error; } - GITERR_CHECK_ALLOC_ADD(offset, 1); - summary = git__calloc(offset + 1, sizeof(char)); + GITERR_CHECK_ALLOC_ADD(&allocsize, offset, 1); + summary = git__calloc(allocsize, sizeof(char)); GITERR_CHECK_ALLOC(summary); strncpy(summary, opts->summary, offset); diff --git a/src/diff_driver.c b/src/diff_driver.c index 67f1c591d..e4d9a0699 100644 --- a/src/diff_driver.c +++ b/src/diff_driver.c @@ -163,12 +163,13 @@ static int diff_driver_alloc( { git_diff_driver *driver; size_t driverlen = sizeof(git_diff_driver), - namelen = strlen(name); + namelen = strlen(name), + alloclen; - GITERR_CHECK_ALLOC_ADD(driverlen, namelen); - GITERR_CHECK_ALLOC_ADD(driverlen + namelen, 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, driverlen, namelen); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); - driver = git__calloc(1, driverlen + namelen + 1); + driver = git__calloc(1, alloclen); GITERR_CHECK_ALLOC(driver); memcpy(driver->name, name, namelen); diff --git a/src/diff_patch.c b/src/diff_patch.c index f5eecae66..1c4c0e8b8 100644 --- a/src/diff_patch.c +++ b/src/diff_patch.c @@ -388,16 +388,11 @@ static int diff_patch_with_delta_alloc( diff_patch_with_delta *pd; size_t old_len = *old_path ? strlen(*old_path) : 0; size_t new_len = *new_path ? strlen(*new_path) : 0; - size_t alloc_len = sizeof(*pd); + size_t alloc_len; - GITERR_CHECK_ALLOC_ADD(alloc_len, old_len); - alloc_len += old_len; - - GITERR_CHECK_ALLOC_ADD(alloc_len, new_len); - alloc_len += new_len; - - GITERR_CHECK_ALLOC_ADD(alloc_len, 2); - alloc_len += 2; + GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(*pd), old_len); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, new_len); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 2); *out = pd = git__calloc(1, alloc_len); GITERR_CHECK_ALLOC(pd); diff --git a/src/diff_tform.c b/src/diff_tform.c index cad1356c3..8ee568cf4 100644 --- a/src/diff_tform.c +++ b/src/diff_tform.c @@ -811,6 +811,7 @@ int git_diff_find_similar( size_t num_deltas, num_srcs = 0, num_tgts = 0; size_t tried_srcs = 0, tried_tgts = 0; size_t num_rewrites = 0, num_updates = 0, num_bumped = 0; + size_t sigcache_size; void **sigcache = NULL; /* cache of similarity metric file signatures */ diff_find_match *tgt2src = NULL; diff_find_match *src2tgt = NULL; @@ -831,8 +832,8 @@ int git_diff_find_similar( if ((opts.flags & GIT_DIFF_FIND_ALL) == 0) goto cleanup; - GITERR_CHECK_ALLOC_MULTIPLY(num_deltas, 2); - sigcache = git__calloc(num_deltas * 2, sizeof(void *)); + GITERR_CHECK_ALLOC_MULTIPLY(&sigcache_size, num_deltas, 2); + sigcache = git__calloc(sigcache_size, sizeof(void *)); GITERR_CHECK_ALLOC(sigcache); /* Label rename sources and targets diff --git a/src/filebuf.c b/src/filebuf.c index 94f2bec32..932b8c7d1 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -194,7 +194,7 @@ static int write_deflate(git_filebuf *file, void *source, size_t len) int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode) { int compression, error = -1; - size_t path_len; + size_t path_len, alloc_len; /* opening an already open buffer is a programming error; * assert that this never happens instead of returning @@ -271,8 +271,8 @@ int git_filebuf_open(git_filebuf *file, const char *path, int flags, mode_t mode GITERR_CHECK_ALLOC(file->path_original); /* create the locking path by appending ".lock" to the original */ - GITERR_CHECK_ALLOC_ADD(path_len, GIT_FILELOCK_EXTLENGTH); - file->path_lock = git__malloc(path_len + GIT_FILELOCK_EXTLENGTH); + GITERR_CHECK_ALLOC_ADD(&alloc_len, path_len, GIT_FILELOCK_EXTLENGTH); + file->path_lock = git__malloc(alloc_len); GITERR_CHECK_ALLOC(file->path_lock); memcpy(file->path_lock, file->path_original, path_len); @@ -408,7 +408,7 @@ int git_filebuf_reserve(git_filebuf *file, void **buffer, size_t len) int git_filebuf_printf(git_filebuf *file, const char *format, ...) { va_list arglist; - size_t space_left, len; + size_t space_left, len, alloclen; int written, res; char *tmp_buffer; @@ -439,8 +439,8 @@ int git_filebuf_printf(git_filebuf *file, const char *format, ...) } while (len + 1 <= space_left); - if (GIT_ALLOC_OVERFLOW_ADD(len, 1) || - !(tmp_buffer = git__malloc(len + 1))) { + if (GIT_ADD_SIZET_OVERFLOW(&alloclen, len, 1) || + !(tmp_buffer = git__malloc(alloclen))) { file->last_error = BUFERR_MEM; return -1; } diff --git a/src/fileops.c b/src/fileops.c index 420ed70a2..09a8f5d4a 100644 --- a/src/fileops.c +++ b/src/fileops.c @@ -124,6 +124,7 @@ mode_t git_futils_canonical_mode(mode_t raw_mode) int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) { ssize_t read_size = 0; + size_t alloc_len; git_buf_clear(buf); @@ -132,8 +133,8 @@ int git_futils_readbuffer_fd(git_buf *buf, git_file fd, size_t len) return -1; } - GITERR_CHECK_ALLOC_ADD(len, 1); - if (git_buf_grow(buf, len + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&alloc_len, len, 1); + if (git_buf_grow(buf, alloc_len) < 0) return -1; /* p_read loops internally to read len bytes */ @@ -455,7 +456,13 @@ int git_futils_mkdir_ext( } if (opts->dir_map && opts->pool) { - char *cache_path = git_pool_malloc(opts->pool, make_path.size + 1); + char *cache_path; + size_t alloc_size; + + GITERR_CHECK_ALLOC_ADD(&alloc_size, make_path.size, 1); + if (!git__is_uint32(alloc_size)) + return -1; + cache_path = git_pool_malloc(opts->pool, (uint32_t)alloc_size); GITERR_CHECK_ALLOC(cache_path); memcpy(cache_path, make_path.ptr, make_path.size + 1); @@ -715,9 +722,10 @@ static int cp_link(const char *from, const char *to, size_t link_size) int error = 0; ssize_t read_len; char *link_data; + size_t alloc_size; - GITERR_CHECK_ALLOC_ADD(link_size, 1); - link_data = git__malloc(link_size + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_size, link_size, 1); + link_data = git__malloc(alloc_size); GITERR_CHECK_ALLOC(link_data); read_len = p_readlink(from, link_data, link_size); diff --git a/src/filter.c b/src/filter.c index d5c669f01..7b54a76c0 100644 --- a/src/filter.c +++ b/src/filter.c @@ -245,14 +245,9 @@ int git_filter_register( if (filter_def_scan_attrs(&attrs, &nattr, &nmatch, filter->attributes) < 0) return -1; - GITERR_CHECK_ALLOC_MULTIPLY(nattr, 2); - alloc_len = nattr * 2; - - GITERR_CHECK_ALLOC_MULTIPLY(alloc_len, sizeof(char *)); - alloc_len *= sizeof(char *); - - GITERR_CHECK_ALLOC_ADD(alloc_len, sizeof(git_filter_def)); - alloc_len += sizeof(git_filter_def); + GITERR_CHECK_ALLOC_MULTIPLY(&alloc_len, nattr, 2); + GITERR_CHECK_ALLOC_MULTIPLY(&alloc_len, alloc_len, sizeof(char *)); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, sizeof(git_filter_def)); fdef = git__calloc(1, alloc_len); GITERR_CHECK_ALLOC(fdef); @@ -385,12 +380,12 @@ static int filter_list_new( git_filter_list **out, const git_filter_source *src) { git_filter_list *fl = NULL; - size_t pathlen = src->path ? strlen(src->path) : 0; + size_t pathlen = src->path ? strlen(src->path) : 0, alloclen; - GITERR_CHECK_ALLOC_ADD(sizeof(git_filter_list), pathlen); - GITERR_CHECK_ALLOC_ADD(sizeof(git_filter_list) + pathlen, 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(git_filter_list), pathlen); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); - fl = git__calloc(1, sizeof(git_filter_list) + pathlen + 1); + fl = git__calloc(1, alloclen); GITERR_CHECK_ALLOC(fl); if (src->path) diff --git a/src/index.c b/src/index.c index 35f512ca4..75700a719 100644 --- a/src/index.c +++ b/src/index.c @@ -770,7 +770,7 @@ static int index_entry_create( git_repository *repo, const char *path) { - size_t pathlen = strlen(path); + size_t pathlen = strlen(path), alloclen; struct entry_internal *entry; if (!git_path_isvalid(repo, path, @@ -779,9 +779,9 @@ static int index_entry_create( return -1; } - GITERR_CHECK_ALLOC_ADD(sizeof(struct entry_internal), pathlen); - GITERR_CHECK_ALLOC_ADD(sizeof(struct entry_internal) + pathlen, 1); - entry = git__calloc(1, sizeof(struct entry_internal) + pathlen + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(struct entry_internal), pathlen); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); + entry = git__calloc(1, alloclen); GITERR_CHECK_ALLOC(entry); entry->pathlen = pathlen; @@ -829,14 +829,15 @@ static int index_entry_init( static git_index_reuc_entry *reuc_entry_alloc(const char *path) { size_t pathlen = strlen(path), - structlen = sizeof(struct reuc_entry_internal); + structlen = sizeof(struct reuc_entry_internal), + alloclen; struct reuc_entry_internal *entry; - if (GIT_ALLOC_OVERFLOW_ADD(structlen, pathlen) || - GIT_ALLOC_OVERFLOW_ADD(structlen + pathlen, 1)) + if (GIT_ADD_SIZET_OVERFLOW(&alloclen, structlen, pathlen) || + GIT_ADD_SIZET_OVERFLOW(&alloclen, alloclen, 1)) return NULL; - entry = git__calloc(1, structlen + pathlen + 1); + entry = git__calloc(1, alloclen); if (!entry) return NULL; diff --git a/src/integer.h b/src/integer.h index a9ed10aae..a4abe2bd1 100644 --- a/src/integer.h +++ b/src/integer.h @@ -35,6 +35,13 @@ GIT_INLINE(int) git__is_ulong(git_off_t p) return p == (git_off_t)r; } +/** @return true if p fits into the range of an int */ +GIT_INLINE(int) git__is_int(long long p) +{ + int r = (int)p; + return p == (long long)r; +} + /** * Sets `one + two` into `out`, unless the arithmetic would overflow. * @return true if the result fits in a `uint64_t`, false on overflow. @@ -42,10 +49,9 @@ GIT_INLINE(int) git__is_ulong(git_off_t p) GIT_INLINE(bool) git__add_uint64_overflow(uint64_t *out, uint64_t one, uint64_t two) { if (UINT64_MAX - one < two) - return false; - if (out) - *out = one + two; - return true; + return true; + *out = one + two; + return false; } /** @@ -55,10 +61,9 @@ GIT_INLINE(bool) git__add_uint64_overflow(uint64_t *out, uint64_t one, uint64_t GIT_INLINE(bool) git__add_sizet_overflow(size_t *out, size_t one, size_t two) { if (SIZE_MAX - one < two) - return false; - if (out) - *out = one + two; - return true; + return true; + *out = one + two; + return false; } /** @@ -68,10 +73,9 @@ GIT_INLINE(bool) git__add_sizet_overflow(size_t *out, size_t one, size_t two) GIT_INLINE(bool) git__multiply_sizet_overflow(size_t *out, size_t one, size_t two) { if (one && SIZE_MAX / one < two) - return false; - if (out) - *out = one * two; - return true; + return true; + *out = one * two; + return false; } #endif /* INCLUDE_integer_h__ */ diff --git a/src/iterator.c b/src/iterator.c index e90cf30ff..9ddacebd1 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -344,11 +344,8 @@ static int tree_iterator__push_frame(tree_iterator *ti) for (i = head->current; i < head->next; ++i) n_entries += git_tree_entrycount(head->entries[i]->tree); - GITERR_CHECK_ALLOC_MULTIPLY(sizeof(tree_iterator_entry *), n_entries); - alloclen = sizeof(tree_iterator_entry *) * n_entries; - - GITERR_CHECK_ALLOC_ADD(alloclen, sizeof(tree_iterator_frame)); - alloclen += sizeof(tree_iterator_frame); + GITERR_CHECK_ALLOC_MULTIPLY(&alloclen, sizeof(tree_iterator_entry *), n_entries); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, sizeof(tree_iterator_frame)); tf = git__calloc(1, alloclen); GITERR_CHECK_ALLOC(tf); diff --git a/src/merge.c b/src/merge.c index 25d7bd7aa..e4b60c847 100644 --- a/src/merge.c +++ b/src/merge.c @@ -1169,8 +1169,7 @@ int git_merge_diff_list__find_renames( goto done; if (diff_list->conflicts.length <= opts->target_limit) { - GITERR_CHECK_ALLOC_MULTIPLY(diff_list->conflicts.length, 3); - cache_size = diff_list->conflicts.length * 3; + GITERR_CHECK_ALLOC_MULTIPLY(&cache_size, diff_list->conflicts.length, 3); cache = git__calloc(cache_size, sizeof(void *)); GITERR_CHECK_ALLOC(cache); @@ -2224,13 +2223,13 @@ static int merge_ancestor_head( size_t their_heads_len) { git_oid *oids, ancestor_oid; - size_t i; + size_t i, alloc_len; int error = 0; assert(repo && our_head && their_heads); - GITERR_CHECK_ALLOC_ADD(their_heads_len, 1); - oids = git__calloc(their_heads_len + 1, sizeof(git_oid)); + GITERR_CHECK_ALLOC_ADD(&alloc_len, their_heads_len, 1); + oids = git__calloc(alloc_len, sizeof(git_oid)); GITERR_CHECK_ALLOC(oids); git_oid_cpy(&oids[0], git_commit_id(our_head->commit)); diff --git a/src/odb.c b/src/odb.c index fcb21c1ce..b1d606b4d 100644 --- a/src/odb.c +++ b/src/odb.c @@ -216,30 +216,31 @@ int git_odb__hashfd_filtered( int git_odb__hashlink(git_oid *out, const char *path) { struct stat st; - size_t size; + int size; int result; if (git_path_lstat(path, &st) < 0) return -1; - if (!git__is_sizet(st.st_size)) { + if (!git__is_int(st.st_size) || (int)st.st_size < 0) { giterr_set(GITERR_FILESYSTEM, "File size overflow for 32-bit systems"); return -1; } - size = (size_t)st.st_size; + size = (int)st.st_size; if (S_ISLNK(st.st_mode)) { char *link_data; - ssize_t read_len; + int read_len; + size_t alloc_size; - GITERR_CHECK_ALLOC_ADD(size, 1); - link_data = git__malloc(size + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_size, size, 1); + link_data = git__malloc(alloc_size); GITERR_CHECK_ALLOC(link_data); read_len = p_readlink(path, link_data, size); link_data[size] = '\0'; - if (read_len != (ssize_t)size) { + if (read_len != size) { giterr_set(GITERR_OS, "Failed to read symlink data for '%s'", path); git__free(link_data); return -1; diff --git a/src/odb_loose.c b/src/odb_loose.c index e43b261fa..bfd95588b 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -63,10 +63,12 @@ typedef struct { static int object_file_name( git_buf *name, const loose_backend *be, const git_oid *id) { + size_t alloclen; + /* expand length for object root + 40 hex sha1 chars + 2 * '/' + '\0' */ - GITERR_CHECK_ALLOC_ADD(be->objects_dirlen, GIT_OID_HEXSZ); - GITERR_CHECK_ALLOC_ADD(be->objects_dirlen + GIT_OID_HEXSZ, 3); - if (git_buf_grow(name, be->objects_dirlen + GIT_OID_HEXSZ + 3) < 0) + GITERR_CHECK_ALLOC_ADD(&alloclen, be->objects_dirlen, GIT_OID_HEXSZ); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 3); + if (git_buf_grow(name, alloclen) < 0) return -1; git_buf_set(name, be->objects_dir, be->objects_dirlen); @@ -263,15 +265,15 @@ static int inflate_buffer(void *in, size_t inlen, void *out, size_t outlen) static void *inflate_tail(z_stream *s, void *hb, size_t used, obj_hdr *hdr) { unsigned char *buf, *head = hb; - size_t tail; + size_t tail, alloc_size; /* * allocate a buffer to hold the inflated data and copy the * initial sequence of inflated data from the tail of the * head buffer, if any. */ - if (GIT_ALLOC_OVERFLOW_ADD(hdr->size, 1) || - (buf = git__malloc(hdr->size + 1)) == NULL) { + if (GIT_ADD_SIZET_OVERFLOW(&alloc_size, hdr->size, 1) || + (buf = git__malloc(alloc_size)) == NULL) { inflateEnd(s); return NULL; } @@ -309,7 +311,7 @@ static int inflate_packlike_loose_disk_obj(git_rawobj *out, git_buf *obj) { unsigned char *in, *buf; obj_hdr hdr; - size_t len, used; + size_t len, used, alloclen; /* * read the object header, which is an (uncompressed) @@ -324,8 +326,8 @@ static int inflate_packlike_loose_disk_obj(git_rawobj *out, git_buf *obj) /* * allocate a buffer and inflate the data into it */ - GITERR_CHECK_ALLOC_ADD(hdr.size, 1); - buf = git__malloc(hdr.size + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, hdr.size, 1); + buf = git__malloc(alloclen); GITERR_CHECK_ALLOC(buf); in = ((unsigned char *)obj->ptr) + used; @@ -519,14 +521,14 @@ static int locate_object_short_oid( size_t len) { char *objects_dir = backend->objects_dir; - size_t dir_len = strlen(objects_dir); + size_t dir_len = strlen(objects_dir), alloc_len; loose_locate_object_state state; int error; /* prealloc memory for OBJ_DIR/xx/xx..38x..xx */ - GITERR_CHECK_ALLOC_ADD(dir_len, GIT_OID_HEXSZ); - GITERR_CHECK_ALLOC_ADD(dir_len + GIT_OID_HEXSZ, 3); - if (git_buf_grow(object_location, dir_len + 3 + GIT_OID_HEXSZ) < 0) + GITERR_CHECK_ALLOC_ADD(&alloc_len, dir_len, GIT_OID_HEXSZ); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 3); + if (git_buf_grow(object_location, alloc_len) < 0) return -1; git_buf_set(object_location, objects_dir, dir_len); @@ -569,11 +571,11 @@ static int locate_object_short_oid( return error; /* Update the location according to the oid obtained */ - GITERR_CHECK_ALLOC_ADD(dir_len, GIT_OID_HEXSZ); - GITERR_CHECK_ALLOC_ADD(dir_len + GIT_OID_HEXSZ, 2); + GITERR_CHECK_ALLOC_ADD(&alloc_len, dir_len, GIT_OID_HEXSZ); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 2); git_buf_truncate(object_location, dir_len); - if (git_buf_grow(object_location, dir_len + GIT_OID_HEXSZ + 2) < 0) + if (git_buf_grow(object_location, alloc_len) < 0) return -1; git_oid_pathfmt(object_location->ptr + dir_len, res_oid); @@ -930,15 +932,15 @@ int git_odb_backend_loose( unsigned int file_mode) { loose_backend *backend; - size_t objects_dirlen; + size_t objects_dirlen, alloclen; assert(backend_out && objects_dir); objects_dirlen = strlen(objects_dir); - GITERR_CHECK_ALLOC_ADD(sizeof(loose_backend), objects_dirlen); - GITERR_CHECK_ALLOC_ADD(sizeof(loose_backend) + objects_dirlen, 2); - backend = git__calloc(1, sizeof(loose_backend) + objects_dirlen + 2); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(loose_backend), objects_dirlen); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 2); + backend = git__calloc(1, alloclen); GITERR_CHECK_ALLOC(backend); backend->parent.version = GIT_ODB_BACKEND_VERSION; diff --git a/src/odb_mempack.c b/src/odb_mempack.c index a71d8db4b..32bc84442 100644 --- a/src/odb_mempack.c +++ b/src/odb_mempack.c @@ -38,6 +38,7 @@ static int impl__write(git_odb_backend *_backend, const git_oid *oid, const void struct memory_packer_db *db = (struct memory_packer_db *)_backend; struct memobject *obj = NULL; khiter_t pos; + size_t alloc_len; int rval; pos = kh_put(oid, db->objects, oid, &rval); @@ -47,8 +48,8 @@ static int impl__write(git_odb_backend *_backend, const git_oid *oid, const void if (rval == 0) return 0; - GITERR_CHECK_ALLOC_ADD(sizeof(struct memobject), len); - obj = git__malloc(sizeof(struct memobject) + len); + GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(struct memobject), len); + obj = git__malloc(alloc_len); GITERR_CHECK_ALLOC(obj); memcpy(obj->data, data, len); diff --git a/src/pack-objects.c b/src/pack-objects.c index 8236ef9f3..67d6125ff 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -202,9 +202,8 @@ int git_packbuilder_insert(git_packbuilder *pb, const git_oid *oid, return 0; if (pb->nr_objects >= pb->nr_alloc) { - GITERR_CHECK_ALLOC_ADD(pb->nr_alloc, 1024); - GITERR_CHECK_ALLOC_MULTIPLY(pb->nr_alloc + 1024, 3 / 2); - newsize = (pb->nr_alloc + 1024) * 3 / 2; + GITERR_CHECK_ALLOC_ADD(&newsize, pb->nr_alloc, 1024); + GITERR_CHECK_ALLOC_MULTIPLY(&newsize, newsize, 3 / 2); if (!git__is_uint32(newsize)) { giterr_set(GITERR_NOMEMORY, "Packfile too large to fit in memory."); @@ -833,7 +832,7 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg, trg_object->delta_data = NULL; } if (delta_cacheable(pb, src_size, trg_size, delta_size)) { - if (!git__add_uint64_overflow(&pb->delta_cache_size, pb->delta_cache_size, delta_size)) + if (git__add_uint64_overflow(&pb->delta_cache_size, pb->delta_cache_size, delta_size)) return -1; git_packbuilder__cache_unlock(pb); diff --git a/src/pack.c b/src/pack.c index d475b28ee..26a6036c2 100644 --- a/src/pack.c +++ b/src/pack.c @@ -624,7 +624,7 @@ int git_packfile_unpack( struct pack_chain_elem *elem = NULL, *stack; git_pack_cache_entry *cached = NULL; struct pack_chain_elem small_stack[SMALL_STACK_SIZE]; - size_t stack_size = 0, elem_pos; + size_t stack_size = 0, elem_pos, alloclen; git_otype base_type; /* @@ -684,8 +684,8 @@ int git_packfile_unpack( if (cached && stack_size == 1) { void *data = obj->data; - GITERR_CHECK_ALLOC_ADD(obj->len, 1); - obj->data = git__malloc(obj->len + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, obj->len, 1); + obj->data = git__malloc(alloclen); GITERR_CHECK_ALLOC(obj->data); memcpy(obj->data, data, obj->len + 1); @@ -840,17 +840,18 @@ int packfile_unpack_compressed( size_t size, git_otype type) { + size_t buf_size; int st; z_stream stream; unsigned char *buffer, *in; - GITERR_CHECK_ALLOC_ADD(size, 1); - buffer = git__calloc(1, size + 1); + GITERR_CHECK_ALLOC_ADD(&buf_size, size, 1); + buffer = git__calloc(1, buf_size); GITERR_CHECK_ALLOC(buffer); memset(&stream, 0, sizeof(stream)); stream.next_out = buffer; - stream.avail_out = (uInt)size + 1; + stream.avail_out = (uInt)buf_size; stream.zalloc = use_git_alloc; stream.zfree = use_git_free; @@ -1089,17 +1090,17 @@ int git_packfile_alloc(struct git_pack_file **pack_out, const char *path) { struct stat st; struct git_pack_file *p; - size_t path_len = path ? strlen(path) : 0; + size_t path_len = path ? strlen(path) : 0, alloc_len; *pack_out = NULL; if (path_len < strlen(".idx")) return git_odb__error_notfound("invalid packfile path", NULL); - GITERR_CHECK_ALLOC_ADD(sizeof(*p), path_len); - GITERR_CHECK_ALLOC_ADD(sizeof(*p) + path_len, 2); + GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(*p), path_len); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 2); - p = git__calloc(1, sizeof(*p) + path_len + 2); + p = git__calloc(1, alloc_len); GITERR_CHECK_ALLOC(p); memcpy(p->pack_name, path, path_len + 1); diff --git a/src/path.c b/src/path.c index 3dbd7187b..64c7b7e12 100644 --- a/src/path.c +++ b/src/path.c @@ -620,11 +620,12 @@ static bool _check_dir_contents( bool result; size_t dir_size = git_buf_len(dir); size_t sub_size = strlen(sub); + size_t alloc_size; /* leave base valid even if we could not make space for subdir */ - if (GIT_ALLOC_OVERFLOW_ADD(dir_size, sub_size) || - GIT_ALLOC_OVERFLOW_ADD(dir_size + sub_size, 2) || - git_buf_try_grow(dir, dir_size + sub_size + 2, false, false) < 0) + if (GIT_ADD_SIZET_OVERFLOW(&alloc_size, dir_size, sub_size) || + GIT_ADD_SIZET_OVERFLOW(&alloc_size, alloc_size, 2) || + git_buf_try_grow(dir, alloc_size, false, false) < 0) return false; /* save excursion */ @@ -786,7 +787,7 @@ int git_path_cmp( int git_path_make_relative(git_buf *path, const char *parent) { const char *p, *q, *p_dirsep, *q_dirsep; - size_t plen = path->size, newlen, depth = 1, i, offset; + size_t plen = path->size, newlen, alloclen, depth = 1, i, offset; for (p_dirsep = p = path->ptr, q_dirsep = q = parent; *p && *q; p++, q++) { if (*p == '/' && *q == '/') { @@ -824,14 +825,14 @@ int git_path_make_relative(git_buf *path, const char *parent) for (; (q = strchr(q, '/')) && *(q + 1); q++) depth++; - GITERR_CHECK_ALLOC_MULTIPLY(depth, 3); - GITERR_CHECK_ALLOC_ADD(depth * 3, plen); - GITERR_CHECK_ALLOC_ADD(depth * 3, 1); - newlen = (depth * 3) + plen; + GITERR_CHECK_ALLOC_MULTIPLY(&newlen, depth, 3); + GITERR_CHECK_ALLOC_ADD(&newlen, newlen, plen); + + GITERR_CHECK_ALLOC_ADD(&alloclen, newlen, 1); /* save the offset as we might realllocate the pointer */ offset = p - path->ptr; - if (git_buf_try_grow(path, newlen + 1, 1, 0) < 0) + if (git_buf_try_grow(path, alloclen, 1, 0) < 0) return -1; p = path->ptr + offset; @@ -876,7 +877,7 @@ void git_path_iconv_clear(git_path_iconv_t *ic) int git_path_iconv(git_path_iconv_t *ic, char **in, size_t *inlen) { char *nfd = *in, *nfc; - size_t nfdlen = *inlen, nfclen, wantlen = nfdlen, rv; + size_t nfdlen = *inlen, nfclen, wantlen = nfdlen, alloclen, rv; int retry = 1; if (!ic || ic->map == (iconv_t)-1 || @@ -886,8 +887,8 @@ int git_path_iconv(git_path_iconv_t *ic, char **in, size_t *inlen) git_buf_clear(&ic->buf); while (1) { - GITERR_CHECK_ALLOC_ADD(wantlen, 1); - if (git_buf_grow(&ic->buf, wantlen + 1) < 0) + GITERR_CHECK_ALLOC_ADD(&alloclen, wantlen, 1); + if (git_buf_grow(&ic->buf, alloclen) < 0) return -1; nfc = ic->buf.ptr + ic->buf.size; @@ -1072,21 +1073,13 @@ static int entry_path_alloc( size_t alloc_extra) { int need_slash = (path_len > 0 && path[path_len-1] != '/') ? 1 : 0; - size_t alloc_size = path_len; + size_t alloc_size; char *entry_path; - GITERR_CHECK_ALLOC_ADD(alloc_size, de_len); - alloc_size += de_len; - - GITERR_CHECK_ALLOC_ADD(alloc_size, need_slash); - alloc_size += need_slash; - - GITERR_CHECK_ALLOC_ADD(alloc_size, 1); - alloc_size++; - - GITERR_CHECK_ALLOC_ADD(alloc_size, alloc_extra); - alloc_size += alloc_extra; - + GITERR_CHECK_ALLOC_ADD(&alloc_size, path_len, de_len); + GITERR_CHECK_ALLOC_ADD(&alloc_size, alloc_size, need_slash); + GITERR_CHECK_ALLOC_ADD(&alloc_size, alloc_size, 1); + GITERR_CHECK_ALLOC_ADD(&alloc_size, alloc_size, alloc_extra); entry_path = git__calloc(1, alloc_size); GITERR_CHECK_ALLOC(entry_path); diff --git a/src/pool.c b/src/pool.c index 919e13d08..c93d78182 100644 --- a/src/pool.c +++ b/src/pool.c @@ -107,21 +107,22 @@ static void pool_insert_page(git_pool *pool, git_pool_page *page) static void *pool_alloc_page(git_pool *pool, uint32_t size) { git_pool_page *page; - uint32_t alloc_size; + uint32_t new_page_size; + size_t alloc_size; if (size <= pool->page_size) - alloc_size = pool->page_size; + new_page_size = pool->page_size; else { - alloc_size = size; + new_page_size = size; pool->has_large_page_alloc = 1; } - if (GIT_ALLOC_OVERFLOW_ADD(alloc_size, sizeof(git_pool_page)) || - !(page = git__calloc(1, alloc_size + sizeof(git_pool_page)))) + if (GIT_ADD_SIZET_OVERFLOW(&alloc_size, new_page_size, sizeof(git_pool_page)) || + !(page = git__calloc(1, alloc_size))) return NULL; - page->size = alloc_size; - page->avail = alloc_size - size; + page->size = new_page_size; + page->avail = new_page_size - size; if (page->avail > 0) pool_insert_page(pool, page); diff --git a/src/refs.c b/src/refs.c index af3190ef9..2e88c26c3 100644 --- a/src/refs.c +++ b/src/refs.c @@ -36,15 +36,13 @@ enum { static git_reference *alloc_ref(const char *name) { - git_reference *ref; - size_t namelen = strlen(name), reflen = sizeof(git_reference); + git_reference *ref = NULL; + size_t namelen = strlen(name), reflen; - if (GIT_ALLOC_OVERFLOW_ADD(reflen, namelen) || - GIT_ALLOC_OVERFLOW_ADD(reflen + namelen, 1) || - (ref = git__calloc(1, reflen + namelen + 1)) == NULL) - return NULL; - - memcpy(ref->name, name, namelen + 1); + if (!GIT_ADD_SIZET_OVERFLOW(&reflen, sizeof(git_reference), namelen) && + !GIT_ADD_SIZET_OVERFLOW(&reflen, reflen, 1) && + (ref = git__calloc(1, reflen)) != NULL) + memcpy(ref->name, name, namelen + 1); return ref; } @@ -96,12 +94,12 @@ git_reference *git_reference__set_name( git_reference *ref, const char *name) { size_t namelen = strlen(name); - size_t reflen = sizeof(git_reference); + size_t reflen; git_reference *rewrite = NULL; - if (!GIT_ALLOC_OVERFLOW_ADD(reflen, namelen) && - !GIT_ALLOC_OVERFLOW_ADD(reflen + namelen, 1) && - (rewrite = git__realloc(ref, reflen + namelen + 1)) != NULL) + if (!GIT_ADD_SIZET_OVERFLOW(&reflen, sizeof(git_reference), namelen) && + !GIT_ADD_SIZET_OVERFLOW(&reflen, reflen, 1) && + (rewrite = git__realloc(ref, reflen)) != NULL) memcpy(rewrite->name, name, namelen + 1); return rewrite; diff --git a/src/sortedcache.c b/src/sortedcache.c index 021f79632..1195ea339 100644 --- a/src/sortedcache.c +++ b/src/sortedcache.c @@ -11,13 +11,13 @@ int git_sortedcache_new( const char *path) { git_sortedcache *sc; - size_t pathlen; + size_t pathlen, alloclen; pathlen = path ? strlen(path) : 0; - GITERR_CHECK_ALLOC_ADD(sizeof(git_sortedcache), pathlen); - GITERR_CHECK_ALLOC_ADD(sizeof(git_sortedcache) + pathlen, 1); - sc = git__calloc(1, sizeof(git_sortedcache) + pathlen + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(git_sortedcache), pathlen); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); + sc = git__calloc(1, alloclen); GITERR_CHECK_ALLOC(sc); if (git_pool_init(&sc->pool, 1, 0) < 0 || diff --git a/src/tag.c b/src/tag.c index b82131401..3b8d684ed 100644 --- a/src/tag.c +++ b/src/tag.c @@ -72,7 +72,7 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) }; unsigned int i; - size_t text_len; + size_t text_len, alloc_len; char *search; if (git_oid__parse(&tag->target, &buffer, buffer_end, "object ") < 0) @@ -117,8 +117,8 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) text_len = search - buffer; - GITERR_CHECK_ALLOC_ADD(text_len, 1); - tag->tag_name = git__malloc(text_len + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_len, text_len, 1); + tag->tag_name = git__malloc(alloc_len); GITERR_CHECK_ALLOC(tag->tag_name); memcpy(tag->tag_name, buffer, text_len); @@ -142,8 +142,8 @@ static int tag_parse(git_tag *tag, const char *buffer, const char *buffer_end) text_len = buffer_end - ++buffer; - GITERR_CHECK_ALLOC_ADD(text_len, 1); - tag->message = git__malloc(text_len + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_len, text_len, 1); + tag->message = git__malloc(alloc_len); GITERR_CHECK_ALLOC(tag->message); memcpy(tag->message, buffer, text_len); diff --git a/src/transports/cred.c b/src/transports/cred.c index 8e5447d18..8163d3115 100644 --- a/src/transports/cred.c +++ b/src/transports/cred.c @@ -306,15 +306,15 @@ int git_cred_default_new(git_cred **cred) int git_cred_username_new(git_cred **cred, const char *username) { git_cred_username *c; - size_t len; + size_t len, allocsize; assert(cred); len = strlen(username); - GITERR_CHECK_ALLOC_ADD(sizeof(git_cred_username), len); - GITERR_CHECK_ALLOC_ADD(sizeof(git_cred_username) + len, 1); - c = git__malloc(sizeof(git_cred_username) + len + 1); + GITERR_CHECK_ALLOC_ADD(&allocsize, sizeof(git_cred_username), len); + GITERR_CHECK_ALLOC_ADD(&allocsize, allocsize, 1); + c = git__malloc(allocsize); GITERR_CHECK_ALLOC(c); c->parent.credtype = GIT_CREDTYPE_USERNAME; diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index 11c6215ff..d214c9fa5 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -102,10 +102,11 @@ static int pack_pkt(git_pkt **out) static int comment_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_comment *pkt; + size_t alloclen; - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_comment), len); - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_comment) + len, 1); - pkt = git__malloc(sizeof(git_pkt_comment) + len + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(git_pkt_comment), len); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); + pkt = git__malloc(alloclen); GITERR_CHECK_ALLOC(pkt); pkt->type = GIT_PKT_COMMENT; @@ -120,14 +121,15 @@ static int comment_pkt(git_pkt **out, const char *line, size_t len) static int err_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_err *pkt; + size_t alloclen; /* Remove "ERR " from the line */ line += 4; len -= 4; - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress), len); - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress) + len, 1); - pkt = git__malloc(sizeof(git_pkt_err) + len + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(git_pkt_progress), len); + GITERR_CHECK_ALLOC_ADD(&alloclen, alloclen, 1); + pkt = git__malloc(alloclen); GITERR_CHECK_ALLOC(pkt); pkt->type = GIT_PKT_ERR; @@ -143,12 +145,13 @@ static int err_pkt(git_pkt **out, const char *line, size_t len) static int data_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_data *pkt; + size_t alloclen; line++; len--; - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress), len); - pkt = git__malloc(sizeof(git_pkt_data) + len); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(git_pkt_progress), len); + pkt = git__malloc(alloclen); GITERR_CHECK_ALLOC(pkt); pkt->type = GIT_PKT_DATA; @@ -163,12 +166,13 @@ static int data_pkt(git_pkt **out, const char *line, size_t len) static int sideband_progress_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_progress *pkt; + size_t alloclen; line++; len--; - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_progress), len); - pkt = git__malloc(sizeof(git_pkt_progress) + len); + GITERR_CHECK_ALLOC_ADD(&alloclen, sizeof(git_pkt_progress), len); + pkt = git__malloc(alloclen); GITERR_CHECK_ALLOC(pkt); pkt->type = GIT_PKT_PROGRESS; @@ -183,13 +187,14 @@ static int sideband_progress_pkt(git_pkt **out, const char *line, size_t len) static int sideband_error_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_err *pkt; + size_t alloc_len; line++; len--; - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_err), len); - GITERR_CHECK_ALLOC_ADD(sizeof(git_pkt_err) + len, 1); - pkt = git__malloc(sizeof(git_pkt_err) + len + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(git_pkt_err), len); + GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 1); + pkt = git__malloc(alloc_len); GITERR_CHECK_ALLOC(pkt); pkt->type = GIT_PKT_ERR; @@ -209,6 +214,7 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len) { int error; git_pkt_ref *pkt; + size_t alloclen; pkt = git__malloc(sizeof(git_pkt_ref)); GITERR_CHECK_ALLOC(pkt); @@ -232,8 +238,8 @@ static int ref_pkt(git_pkt **out, const char *line, size_t len) if (line[len - 1] == '\n') --len; - GITERR_CHECK_ALLOC_ADD(len, 1); - pkt->head.name = git__malloc(len + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, len, 1); + pkt->head.name = git__malloc(alloclen); GITERR_CHECK_ALLOC(pkt->head.name); memcpy(pkt->head.name, line, len); @@ -255,6 +261,7 @@ static int ok_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_ok *pkt; const char *ptr; + size_t alloc_len; pkt = git__malloc(sizeof(*pkt)); GITERR_CHECK_ALLOC(pkt); @@ -268,8 +275,8 @@ static int ok_pkt(git_pkt **out, const char *line, size_t len) } len = ptr - line; - GITERR_CHECK_ALLOC_ADD(len, 1); - pkt->ref = git__malloc(len + 1); + GITERR_CHECK_ALLOC_ADD(&alloc_len, len, 1); + pkt->ref = git__malloc(alloc_len); GITERR_CHECK_ALLOC(pkt->ref); memcpy(pkt->ref, line, len); @@ -283,6 +290,7 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) { git_pkt_ng *pkt; const char *ptr; + size_t alloclen; pkt = git__malloc(sizeof(*pkt)); GITERR_CHECK_ALLOC(pkt); @@ -296,8 +304,8 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) } len = ptr - line; - GITERR_CHECK_ALLOC_ADD(len, 1); - pkt->ref = git__malloc(len + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, len, 1); + pkt->ref = git__malloc(alloclen); GITERR_CHECK_ALLOC(pkt->ref); memcpy(pkt->ref, line, len); @@ -310,8 +318,8 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) } len = ptr - line; - GITERR_CHECK_ALLOC_ADD(len, 1); - pkt->msg = git__malloc(len + 1); + GITERR_CHECK_ALLOC_ADD(&alloclen, len, 1); + pkt->msg = git__malloc(alloclen); GITERR_CHECK_ALLOC(pkt->msg); memcpy(pkt->msg, line, len); diff --git a/src/tree.c b/src/tree.c index 573e56447..9fd4e0a07 100644 --- a/src/tree.c +++ b/src/tree.c @@ -84,12 +84,11 @@ int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2) static git_tree_entry *alloc_entry(const char *filename) { git_tree_entry *entry = NULL; - size_t filename_len = strlen(filename), - tree_len = sizeof(git_tree_entry); + size_t filename_len = strlen(filename), tree_len; - if (GIT_ALLOC_OVERFLOW_ADD(tree_len, filename_len) || - GIT_ALLOC_OVERFLOW_ADD(tree_len + filename_len, 1) || - !(entry = git__malloc(tree_len + filename_len + 1))) + if (GIT_ADD_SIZET_OVERFLOW(&tree_len, sizeof(git_tree_entry), filename_len) || + GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, 1) || + !(entry = git__malloc(tree_len))) return NULL; memset(entry, 0x0, sizeof(git_tree_entry)); @@ -207,16 +206,13 @@ void git_tree_entry_free(git_tree_entry *entry) int git_tree_entry_dup(git_tree_entry **dest, const git_tree_entry *source) { - size_t total_size = sizeof(git_tree_entry); + size_t total_size; git_tree_entry *copy; assert(source); - GITERR_CHECK_ALLOC_ADD(total_size, source->filename_len); - total_size += source->filename_len; - - GITERR_CHECK_ALLOC_ADD(total_size, 1); - total_size++; + GITERR_CHECK_ALLOC_ADD(&total_size, sizeof(git_tree_entry), source->filename_len); + GITERR_CHECK_ALLOC_ADD(&total_size, total_size, 1); copy = git__malloc(total_size); GITERR_CHECK_ALLOC(copy); diff --git a/src/util.h b/src/util.h index 86139ef77..38dcae79b 100644 --- a/src/util.h +++ b/src/util.h @@ -59,17 +59,13 @@ GIT_INLINE(char *) git__strdup(const char *str) GIT_INLINE(char *) git__strndup(const char *str, size_t n) { - size_t length = 0; + size_t length = 0, alloclength; char *ptr; length = p_strnlen(str, n); - if (GIT_ALLOC_OVERFLOW_ADD(length, 1)) - return NULL; - - ptr = git__malloc(length + 1); - - if (!ptr) + if (GIT_ADD_SIZET_OVERFLOW(&alloclength, length, 1) || + !(ptr = git__malloc(alloclength))) return NULL; if (length) @@ -84,8 +80,10 @@ GIT_INLINE(char *) git__strndup(const char *str, size_t n) GIT_INLINE(char *) git__substrdup(const char *start, size_t n) { char *ptr; + size_t alloclen; - if (GIT_ALLOC_OVERFLOW_ADD(n, 1) || !(ptr = git__malloc(n+1))) + if (GIT_ADD_SIZET_OVERFLOW(&alloclen, n, 1) || + !(ptr = git__malloc(alloclen))) return NULL; memcpy(ptr, start, n); @@ -107,8 +105,9 @@ GIT_INLINE(void *) git__realloc(void *ptr, size_t size) */ GIT_INLINE(void *) git__reallocarray(void *ptr, size_t nelem, size_t elsize) { - return GIT_ALLOC_OVERFLOW_MULTIPLY(nelem, elsize) ? - NULL : realloc(ptr, nelem * elsize); + size_t newsize; + return GIT_MULTIPLY_SIZET_OVERFLOW(&newsize, nelem, elsize) ? + NULL : realloc(ptr, newsize); } /** diff --git a/src/vector.c b/src/vector.c index 27eafebc8..93d09bb5b 100644 --- a/src/vector.c +++ b/src/vector.c @@ -46,8 +46,7 @@ int git_vector_dup(git_vector *v, const git_vector *src, git_vector_cmp cmp) assert(v && src); - GITERR_CHECK_ALLOC_MULTIPLY(src->length, sizeof(void *)); - bytes = src->length * sizeof(void *); + GITERR_CHECK_ALLOC_MULTIPLY(&bytes, src->length, sizeof(void *)); v->_alloc_size = src->length; v->_cmp = cmp ? cmp : src->_cmp; diff --git a/src/win32/dir.c b/src/win32/dir.c index 7f2a5a56e..c15757085 100644 --- a/src/win32/dir.c +++ b/src/win32/dir.c @@ -11,16 +11,16 @@ git__DIR *git__opendir(const char *dir) { git_win32_path filter_w; git__DIR *new = NULL; - size_t dirlen; + size_t dirlen, alloclen; if (!dir || !git_win32__findfirstfile_filter(filter_w, dir)) return NULL; dirlen = strlen(dir); - if (GIT_ALLOC_OVERFLOW_ADD(sizeof(*new), dirlen) || - GIT_ALLOC_OVERFLOW_ADD(sizeof(*new) + dirlen, 1) || - !(new = git__calloc(1, sizeof(*new) + dirlen + 1))) + if (GIT_ADD_SIZET_OVERFLOW(&alloclen, sizeof(*new), dirlen) || + GIT_ADD_SIZET_OVERFLOW(&alloclen, alloclen, 1) || + !(new = git__calloc(1, alloclen))) return NULL; memcpy(new->dir, dir, dirlen); diff --git a/tests/core/errors.c b/tests/core/errors.c index 11e0bb2bb..a06ec4abc 100644 --- a/tests/core/errors.c +++ b/tests/core/errors.c @@ -112,7 +112,8 @@ void test_core_errors__restore(void) static int test_arraysize_multiply(size_t nelem, size_t size) { - GITERR_CHECK_ALLOC_MULTIPLY(nelem, size); + size_t out; + GITERR_CHECK_ALLOC_MULTIPLY(&out, nelem, size); return 0; } @@ -133,7 +134,8 @@ void test_core_errors__integer_overflow_alloc_multiply(void) static int test_arraysize_add(size_t one, size_t two) { - GITERR_CHECK_ALLOC_ADD(one, two); + size_t out; + GITERR_CHECK_ALLOC_ADD(&out, one, two); return 0; } @@ -152,21 +154,23 @@ void test_core_errors__integer_overflow_alloc_add(void) void test_core_errors__integer_overflow_sets_oom(void) { + size_t out; + giterr_clear(); - cl_assert(!GIT_ALLOC_OVERFLOW_ADD(SIZE_MAX-1, 1)); + cl_assert(!GIT_ADD_SIZET_OVERFLOW(&out, SIZE_MAX-1, 1)); cl_assert_equal_p(NULL, giterr_last()); giterr_clear(); - cl_assert(!GIT_ALLOC_OVERFLOW_ADD(42, 69)); + cl_assert(!GIT_ADD_SIZET_OVERFLOW(&out, 42, 69)); cl_assert_equal_p(NULL, giterr_last()); giterr_clear(); - cl_assert(GIT_ALLOC_OVERFLOW_ADD(SIZE_MAX, SIZE_MAX)); + cl_assert(GIT_ADD_SIZET_OVERFLOW(&out, SIZE_MAX, SIZE_MAX)); cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass); cl_assert_equal_s("Out of memory", giterr_last()->message); giterr_clear(); - cl_assert(GIT_ALLOC_OVERFLOW_MULTIPLY(SIZE_MAX, SIZE_MAX)); + cl_assert(GIT_ADD_SIZET_OVERFLOW(&out, SIZE_MAX, SIZE_MAX)); cl_assert_equal_i(GITERR_NOMEMORY, giterr_last()->klass); cl_assert_equal_s("Out of memory", giterr_last()->message); } From 8aab36a3010372edec71e8c765d4ecfd848c09b6 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 12 Feb 2015 22:14:53 +0000 Subject: [PATCH 12/15] filebuf: use an int for return check --- src/filebuf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/filebuf.c b/src/filebuf.c index 932b8c7d1..a35b59cf0 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -446,10 +446,10 @@ int git_filebuf_printf(git_filebuf *file, const char *format, ...) } va_start(arglist, format); - len = p_vsnprintf(tmp_buffer, len + 1, format, arglist); + written = p_vsnprintf(tmp_buffer, len + 1, format, arglist); va_end(arglist); - if (len < 0) { + if (written < 0) { git__free(tmp_buffer); file->last_error = BUFERR_MEM; return -1; From 16942c6fdaddb819b71b72e53aa4aa691e3c0053 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 12 Feb 2015 17:36:48 -0500 Subject: [PATCH 13/15] integer overflow: use compiler intrinsics if supported gcc and clang support __builtin_add_overflow, use it whenever possible, falling back to our naive routines. --- src/common.h | 5 +++++ src/integer.h | 15 +++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/src/common.h b/src/common.h index 8d1e89064..98109ae3a 100644 --- a/src/common.h +++ b/src/common.h @@ -17,6 +17,11 @@ # define GIT_INLINE(type) static inline type #endif +/** Support for gcc/clang __has_builtin intrinsic */ +#ifndef __has_builtin +# define __has_builtin(x) 0 +#endif + #include #include #include diff --git a/src/integer.h b/src/integer.h index a4abe2bd1..8e86a48a5 100644 --- a/src/integer.h +++ b/src/integer.h @@ -54,6 +54,19 @@ GIT_INLINE(bool) git__add_uint64_overflow(uint64_t *out, uint64_t one, uint64_t return false; } +/* Use clang/gcc compiler intrinsics whenever possible */ +#if (SIZE_MAX == UINT_MAX) && __has_builtin(__builtin_uadd_overflow) +# define git__add_sizet_overflow(out, one, two) \ + __builtin_uadd_overflow(one, two, out) +# define git__multiply_sizet_overflow(out, one, two) + __builtin_umul_overflow(one, two, out) +#elif (SIZE_MAX == ULONG_MAX) && __has_builtin(__builtin_uaddl_overflow) +# define git__add_sizet_overflow(out, one, two) \ + __builtin_uaddl_overflow(one, two, out) +# define git__multiply_sizet_overflow(out, one, two) \ + __builtin_umull_overflow(one, two, out) +#else + /** * Sets `one + two` into `out`, unless the arithmetic would overflow. * @return true if the result fits in a `size_t`, false on overflow. @@ -78,4 +91,6 @@ GIT_INLINE(bool) git__multiply_sizet_overflow(size_t *out, size_t one, size_t tw return false; } +#endif + #endif /* INCLUDE_integer_h__ */ From d97d9559e321ac2ef2ae81a6cfd308a841ef7581 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 12 Feb 2015 23:56:09 -0500 Subject: [PATCH 14/15] buf test: cleanup memory leak --- tests/buf/basic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/buf/basic.c b/tests/buf/basic.c index dcc0916d2..14ea3e7ce 100644 --- a/tests/buf/basic.c +++ b/tests/buf/basic.c @@ -33,6 +33,8 @@ void test_buf_basic__resize_incremental(void) cl_git_pass(git_buf_grow_by(&buf1, 16)); cl_assert_equal_i(5, buf1.size); cl_assert(buf1.asize > 8); + + git_buf_free(&buf1); } void test_buf_basic__printf(void) From 0f07d54b44825399e5d13499328135771c8d0b43 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 13 Feb 2015 09:35:20 -0500 Subject: [PATCH 15/15] pack-objects: unlock the cache on integer overflow --- src/pack-objects.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/pack-objects.c b/src/pack-objects.c index 67d6125ff..f644520ac 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -832,13 +832,14 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg, trg_object->delta_data = NULL; } if (delta_cacheable(pb, src_size, trg_size, delta_size)) { - if (git__add_uint64_overflow(&pb->delta_cache_size, pb->delta_cache_size, delta_size)) - return -1; + bool overflow = git__add_uint64_overflow( + &pb->delta_cache_size, pb->delta_cache_size, delta_size); git_packbuilder__cache_unlock(pb); - trg_object->delta_data = git__realloc(delta_buf, delta_size); - GITERR_CHECK_ALLOC(trg_object->delta_data); + if (overflow || + !(trg_object->delta_data = git__realloc(delta_buf, delta_size))) + return -1; } else { /* create delta when writing the pack */ git_packbuilder__cache_unlock(pb);