From 686569ff334ad47a7ad0c781faa27fa4cd149f5d Mon Sep 17 00:00:00 2001 From: "S. Gilles" Date: Mon, 5 Jun 2017 13:48:00 -0400 Subject: [PATCH] Add a bunch of overflow checks around malloc() --- file-selection.c | 7 +++++++ mutation-finder.c | 27 +++++++++++++++++---------- quiver.c | 19 +++++++++++++++++-- ui-sdl.c | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 86 insertions(+), 14 deletions(-) diff --git a/file-selection.c b/file-selection.c index 2e4b847..b83896e 100644 --- a/file-selection.c +++ b/file-selection.c @@ -77,6 +77,7 @@ static int slurp(FILE *f, char **out) size_t sz = read_size - 1; char *buf = 0; + /* No need to check for overflow; sz + 1 == 1 << 7 */ if (!(buf = malloc(sz + 1))) { ret = errno; perror(L("malloc")); @@ -97,6 +98,12 @@ static int slurp(FILE *f, char **out) sz += read_size; void *newmem = 0; + if (sz + 1 < sz) { + ret = errno = EOVERFLOW; + perror(L("")); + goto done; + } + if (!(newmem = realloc(buf, sz + 1))) { ret = errno; perror(L("realloc")); diff --git a/mutation-finder.c b/mutation-finder.c index fc5667f..aec2c3b 100644 --- a/mutation-finder.c +++ b/mutation-finder.c @@ -140,15 +140,15 @@ static int setup(char *start_file, char *end_file, char **frozen, size_t * must match user input - tuning input so that known prefixes * arise early is sometimes crucial. */ - if (!(averts = malloc(a.v_num * sizeof(*averts)))) { + if (!(averts = calloc(a.v_num, sizeof *averts))) { ret = errno; - perror(L("malloc")); + perror(L("calloc")); goto done; } - if (!(bverts = malloc(b.v_num * sizeof(*bverts)))) { + if (!(bverts = calloc(b.v_num, sizeof *bverts))) { ret = errno; - perror(L("malloc")); + perror(L("calloc")); goto done; } @@ -195,15 +195,15 @@ static int setup(char *start_file, char *end_file, char **frozen, size_t } /* a_to_s[i] = j <=> "vertex i in a is vertex j in out_qstart" */ - if (!(a_to_start = malloc(a.v_num * sizeof(*a_to_start)))) { + if (!(a_to_start = calloc(a.v_num, sizeof(*a_to_start)))) { ret = errno; - perror(L("malloc")); + perror(L("calloc")); goto done; } - if (!(b_to_end = malloc(b.v_num * sizeof(*b_to_end)))) { + if (!(b_to_end = calloc(b.v_num, sizeof(*b_to_end)))) { ret = errno; - perror(L("malloc")); + perror(L("calloc")); goto done; } @@ -427,9 +427,16 @@ int main(int argc, char **argv) } /* Save off qend's vertices. XXX: Why not just USE qend? */ - if (!(target = malloc(sizeof *target * qend.v_num * qend.v_num))) { + if ((qend.v_num * qend.v_num) / qend.v_num != qend.v_num) { + /* Should never happen - isn't qend.v_num capped? */ + ret = errno = EOVERFLOW; + perror(L("")); + goto done; + } + + if (!(target = calloc(qend.v_num * qend.v_num, sizeof *target))) { ret = errno; - perror(L("malloc")); + perror(L("calloc")); goto done; } diff --git a/quiver.c b/quiver.c index a0c6665..24392e2 100644 --- a/quiver.c +++ b/quiver.c @@ -178,6 +178,7 @@ int quiver_add_vertex(struct quiver *q, size_t *out_i, const char *name, { void *newmem = 0; char *newname; + size_t len = 0; if (!q) { IF_NZ_SET(out_errstr, L("invalid quiver")); @@ -186,7 +187,15 @@ int quiver_add_vertex(struct quiver *q, size_t *out_i, const char *name, } if (!name) { - if (!(newname = malloc(snprintf(0, 0, "%zu", q->v_num) + 1))) { + len = snprintf(0, 0, "%zu", q->v_num); + + if (len + 1 < len) { + IF_NZ_SET(out_errstr, L("overflow")); + + return EOVERFLOW; + } + + if (!(newname = malloc(len + 1))) { int sv_err = errno; IF_NZ_SET(out_errstr, L("malloc")); @@ -196,6 +205,11 @@ int quiver_add_vertex(struct quiver *q, size_t *out_i, const char *name, sprintf(newname, "%zu", q->v_num); } else { + /* + * Should be no need to check overflow, either name + * was malloc'd, or came from a file which was read + * in with a length cap. + */ if (!(newname = malloc(strlen(name) + 1))) { int sv_err = errno; @@ -210,7 +224,8 @@ int quiver_add_vertex(struct quiver *q, size_t *out_i, const char *name, if (q->v_num >= q->v_len) { size_t newlen = q->v_len + 8; - if (newlen >= ((size_t) -1) / newlen) { + if (newlen >= ((size_t) -1) / newlen || + (newlen * newlen) / newlen != newlen) { IF_NZ_SET(out_errstr, L("too many vertices")); return EDOM; diff --git a/ui-sdl.c b/ui-sdl.c index 6c31011..b447a1c 100644 --- a/ui-sdl.c +++ b/ui-sdl.c @@ -281,6 +281,12 @@ static int pretty_fraction(struct rational *r, char **out) if (r->q == 1) { size_t len = snprintf(0, 0, "%d", (int) r->p); + if (len + 1 < len) { + ret = errno = EOVERFLOW; + perror(L("")); + goto done; + } + if (!(*out = malloc(len + 1))) { ret = errno; perror(L("malloc")); @@ -293,6 +299,12 @@ static int pretty_fraction(struct rational *r, char **out) size_t len = snprintf(0, 0, "%d/%u", (int) r->p, (unsigned int) r->q); + if (len + 1 < len) { + ret = errno = EOVERFLOW; + perror(L("")); + goto done; + } + if (!(*out = malloc(len + 1))) { ret = errno; perror(L("malloc")); @@ -619,6 +631,12 @@ compute_str: v->name, (int) v->fatness, v->x, v->y); + if (len + 1 < len) { + ret = errno = EOVERFLOW; + perror(L("")); + goto done; + } + if (!(s = malloc(len + 1))) { ret = errno; perror(L("malloc")); @@ -655,6 +673,12 @@ compute_str: i->name, j->name, sij, j->name, i->name, sji); + if (len + 1 < len) { + ret = errno = EOVERFLOW; + perror(L("")); + goto done; + } + if (!(s = malloc(len + 1))) { ret = errno; perror(L("malloc")); @@ -691,11 +715,20 @@ static int render_vertex_names(void) return 0; } + if ((q->v_num * sizeof *vertex_names) / q->v_num != + sizeof *vertex_names) { + errno = EOVERFLOW; + perror(L("")); + vertex_names_len = 0; + + return EOVERFLOW; + } + if (!(vertex_names = realloc(vertex_names, q->v_num * sizeof(*vertex_names)))) { int sv_err = errno; - perror(L("realloc()")); + perror(L("realloc")); vertex_names_len = 0; return sv_err; @@ -762,6 +795,15 @@ static int eq_push(struct ui_event *in) return ENOMEM; } + if ((eq_len * 2) / 2 != eq_len || + (eq_len * 2 * sizeof *eq_buf) / (eq_len * 2) != + sizeof *eq_buf) { + errno = EOVERFLOW; + perror(L("")); + + return EOVERFLOW; + } + if (!(newmem = realloc(eq_buf, (eq_len * 2) * sizeof *eq_buf))) { int sv_err = errno; @@ -789,6 +831,7 @@ int ui_init(struct quiver *i_q) q = i_q; size_t padding = strlen("Filename: "); + /* No need for overflow check, max_input_size is capped */ if (!(input_with_prefix = malloc(max_input_size + padding))) { ret = errno; perror(L("malloc")); @@ -861,7 +904,7 @@ int ui_init(struct quiver *i_q) /* Set up queue for returning data */ if (!(eq_buf = calloc(2, sizeof *eq_buf))) { ret = errno; - perror(L("malloc")); + perror(L("calloc")); goto done; } -- 2.11.4.GIT