From a29bbbe97388bb6f9c9b4df36b448dfe5023363c Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Sat, 29 Oct 2005 05:50:06 +0000 Subject: [PATCH] Fix up potential overflows in size calculations by replacing instances of pool_alloc(p, x * sizeof *y) by pool_malloc(p, x, sizeof *y) everywhere I could find them. Similarly by pool_malloc(), pool_realloc(). (Order is important: pool_alloc(p, sizeof *y, x) will divide by 0 if x is 0.) --- src/ChangeLog | 13 +++++++++++++ src/autorecode.c | 4 ++-- src/frequencies.q | 11 +++++------ src/groff-font.c | 12 ++++++------ src/matrix-data.c | 18 +++++++++--------- src/pfm-read.c | 4 ++-- src/pool.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/pool.h | 3 +++ src/tab.c | 29 +++++++++++++++-------------- 9 files changed, 99 insertions(+), 39 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index ce26823e..cad9304c 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,16 @@ +Fri Oct 28 22:47:48 2005 Ben Pfaff + + Fix up potential overflows in size calculations by replacing + instances of pool_alloc(p, x * sizeof *y) by pool_malloc(p, x, + sizeof *y) everywhere I could find them. Similarly by + pool_malloc(), pool_realloc(). + (Order is important: pool_alloc(p, sizeof *y, x) will divide by 0 + if x is 0.) + + * pool.c: (pool_nalloc) New function. + (pool_nmalloc) New function. + (pool_nrealloc) New function. + Thu Oct 27 11:16:53 WST 2005 John Darrington Separated the definition of a file handle object from the stuff diff --git a/src/autorecode.c b/src/autorecode.c index 259d0956..bf38d922 100644 --- a/src/autorecode.c +++ b/src/autorecode.c @@ -231,7 +231,7 @@ recode (const struct autorecode_pgm *arc) t->h.proc = autorecode_trns_proc; t->h.free = autorecode_trns_free; t->owner = pool; - t->specs = pool_alloc (t->owner, sizeof *t->specs * arc->var_cnt); + t->specs = pool_nalloc (t->owner, arc->var_cnt, sizeof *t->specs); t->spec_cnt = arc->var_cnt; for (i = 0; i < arc->var_cnt; i++) { @@ -357,7 +357,7 @@ autorecode_proc_func (struct ccase *c, void *arc_) vpp = (union value **) hsh_probe (arc->src_values[i], &v); if (*vpp == NULL) { - vp = pool_alloc (arc->src_values_pool, sizeof (union value)); + vp = pool_alloc (arc->src_values_pool, sizeof *vp); if (arc->src_vars[i]->type == NUMERIC) vp->f = v.f; else diff --git a/src/frequencies.q b/src/frequencies.q index 5cff5892..5754be7e 100644 --- a/src/frequencies.q +++ b/src/frequencies.q @@ -835,8 +835,8 @@ frq_custom_variables (struct cmd_frequencies *cmd UNUSED) { vf->tab.min = min; vf->tab.max = max; - vf->tab.vector = pool_alloc (int_pool, - sizeof (struct freq) * (max - min + 1)); + vf->tab.vector = pool_nalloc (int_pool, + max - min + 1, sizeof *vf->tab.vector); } else vf->tab.vector = NULL; @@ -878,7 +878,7 @@ frq_custom_grouped (struct cmd_frequencies *cmd UNUSED) if (nl >= ml) { ml += 16; - dl = pool_realloc (int_pool, dl, ml * sizeof (double)); + dl = pool_nrealloc (int_pool, dl, ml, sizeof *dl); } dl[nl++] = tokval; lex_get (); @@ -949,9 +949,8 @@ add_percentile (double x) if (i >= n_percentiles || tokval != percentiles[i].p) { - percentiles - = pool_realloc (int_pool, percentiles, - (n_percentiles + 1) * sizeof (struct percentile )); + percentiles = pool_nrealloc (int_pool, percentiles, + n_percentiles + 1, sizeof *percentiles); if (i < n_percentiles) memmove (&percentiles[i + 1], &percentiles[i], diff --git a/src/groff-font.c b/src/groff-font.c index 33a0b56b..5df16f43 100644 --- a/src/groff-font.c +++ b/src/groff-font.c @@ -562,8 +562,8 @@ check_deref_space (struct font_desc *font, int min_size) font->deref_size = min_size + 16; if (font->deref_size < 256) font->deref_size = 256; - font->deref = pool_realloc (font->owner, font->deref, - sizeof *font->deref * font->deref_size); + font->deref = pool_nrealloc (font->owner, font->deref, + font->deref_size, sizeof *font->deref); for (; i < font->deref_size; i++) font->deref[i] = -1; } @@ -577,8 +577,8 @@ add_char_metric (struct font_desc *font, struct char_metrics *metrics, int code) if (font->metric_used >= font->metric_size) { font->metric_size += 64; - font->metric = pool_realloc (font->owner, font->metric, - sizeof *font->metric * font->metric_size); + font->metric = pool_nrealloc (font->owner, font->metric, + font->metric_size, sizeof *font->metric); } font->metric[font->metric_used] = metrics; font->deref[code] = font->metric_used++; @@ -615,8 +615,8 @@ add_kern (struct font_desc *font, int ch1, int ch2, int adjust) font->kern_size *= 2; font->kern_max_used = font->kern_size / 2; - font->kern = pool_malloc (font->owner, - sizeof *font->kern * font->kern_size); + font->kern = pool_nmalloc (font->owner, + font->kern_size, sizeof *font->kern); for (i = 0; i < font->kern_size; i++) font->kern[i].ch1 = -1; diff --git a/src/matrix-data.c b/src/matrix-data.c index db6189d1..ee43d82f 100644 --- a/src/matrix-data.c +++ b/src/matrix-data.c @@ -1143,7 +1143,7 @@ matrix_data_read_without_rowtype (struct case_source *source, { int *cp; - nr->data = pool_alloc (mx->container, (PROX + 1) * sizeof *nr->data); + nr->data = pool_nalloc (mx->container, PROX + 1, sizeof *nr->data); { int i; @@ -1166,12 +1166,12 @@ matrix_data_read_without_rowtype (struct case_source *source, int n_vectors = per_factor ? mx->cells : 1; int i; - nr->data[*cp] = pool_alloc (mx->container, - n_vectors * sizeof **nr->data); + nr->data[*cp] = pool_nalloc (mx->container, + n_vectors, sizeof **nr->data); for (i = 0; i < n_vectors; i++) - nr->data[*cp][i] = pool_alloc (mx->container, - n_entries * sizeof ***nr->data); + nr->data[*cp][i] = pool_nalloc (mx->container, + n_entries, sizeof ***nr->data); } } } @@ -1836,8 +1836,8 @@ cache_miss: { struct factor_data *new = pool_alloc (mx->container, sizeof *new); - new->factors = pool_alloc (mx->container, - sizeof *new->factors * mx->n_factors); + new->factors = pool_nalloc (mx->container, + mx->n_factors, sizeof *new->factors); { size_t i; @@ -1888,8 +1888,8 @@ wr_read_indeps (struct wr_aux_data *wr) if (type == 1) n_items *= mx->n_continuous; - c->data[wr->content] = pool_alloc (mx->container, - sizeof **c->data * n_items); + c->data[wr->content] = pool_nalloc (mx->container, + n_items, sizeof **c->data); } cp = &c->data[wr->content][n_rows * mx->n_continuous]; diff --git a/src/pfm-read.c b/src/pfm-read.c index 24f92833..a1ee0550 100644 --- a/src/pfm-read.c +++ b/src/pfm-read.c @@ -492,7 +492,7 @@ read_variables (struct pfm_reader *r, struct dictionary *dict) r->var_cnt = read_int (r); if (r->var_cnt <= 0 || r->var_cnt == NOT_INT) error (r, _("Invalid number of variables %d."), r->var_cnt); - r->widths = pool_alloc (r->pool, sizeof *r->widths * r->var_cnt); + r->widths = pool_nalloc (r->pool, r->var_cnt, sizeof *r->widths); /* Purpose of this value is unknown. It is typically 161. */ read_int (r); @@ -608,7 +608,7 @@ read_value_label (struct pfm_reader *r, struct dictionary *dict) int i; nv = read_int (r); - v = pool_alloc (r->pool, sizeof *v * nv); + v = pool_nalloc (r->pool, nv, sizeof *v); for (i = 0; i < nv; i++) { char name[256]; diff --git a/src/pool.c b/src/pool.c index 9824b9b7..4d34c8fe 100644 --- a/src/pool.c +++ b/src/pool.c @@ -273,6 +273,19 @@ pool_alloc (struct pool *pool, size_t amt) return pool_malloc (pool, amt); } +/* Allocates a memory region N * S bytes in size from POOL and + returns a pointer to the region's start. + N must be nonnegative, S must be positive. + Terminates the program if the memory cannot be obtained, + including the case where N * S overflows the range of size_t. */ +void * +pool_nalloc (struct pool *pool, size_t n, size_t s) +{ + if (xalloc_oversized (n, s)) + xalloc_die (); + return pool_alloc (pool, n * s); +} + /* Allocates SIZE bytes in POOL, copies BUFFER into it, and returns the new copy. */ void * @@ -355,6 +368,21 @@ pool_malloc (struct pool *pool, size_t amt) return xmalloc (amt); } +/* Allocates and returns N elements of S bytes each, to be + managed by POOL. + If POOL is a null pointer, then allocates a normal memory block + with malloc(). + N must be nonnegative, S must be positive. + Terminates the program if the memory cannot be obtained, + including the case where N * S overflows the range of size_t. */ +void * +pool_nmalloc (struct pool *pool, size_t n, size_t s) +{ + if (xalloc_oversized (n, s)) + xalloc_die (); + return pool_malloc (pool, n * s); +} + /* Changes the allocation size of the specified memory block P managed by POOL to AMT bytes and returns a pointer to the beginning of the block. @@ -396,6 +424,22 @@ pool_realloc (struct pool *pool, void *p, size_t amt) return xrealloc (p, amt); } +/* Changes the allocation size of the specified memory block P + managed by POOL to N * S bytes and returns a pointer to the + beginning of the block. + N must be nonnegative, S must be positive. + If POOL is a null pointer, then the block is reallocated in + the usual way with xrealloc(). + Terminates the program if the memory cannot be obtained, + including the case where N * S overflows the range of size_t. */ +void * +pool_nrealloc (struct pool *pool, void *p, size_t n, size_t s) +{ + if (xalloc_oversized (n, s)) + xalloc_die (); + return pool_realloc (pool, p, n * s); +} + /* Frees block P managed by POOL. If POOL is a null pointer, then the block is freed as usual with free(). */ diff --git a/src/pool.h b/src/pool.h index e884ae61..e2e90c26 100644 --- a/src/pool.h +++ b/src/pool.h @@ -40,6 +40,7 @@ void pool_clear (struct pool *); /* Suballocation routines. */ void *pool_alloc (struct pool *, size_t) MALLOC_LIKE; +void *pool_nalloc (struct pool *, size_t n, size_t s) MALLOC_LIKE; void *pool_clone (struct pool *, const void *, size_t) MALLOC_LIKE; char *pool_strdup (struct pool *, const char *) MALLOC_LIKE; char *pool_strndup (struct pool *, const char *, size_t) MALLOC_LIKE; @@ -47,7 +48,9 @@ char *pool_strcat (struct pool *, const char *, ...) MALLOC_LIKE; /* Standard allocation routines. */ void *pool_malloc (struct pool *, size_t) MALLOC_LIKE; +void *pool_nmalloc (struct pool *, size_t n, size_t s) MALLOC_LIKE; void *pool_realloc (struct pool *, void *, size_t); +void *pool_nrealloc (struct pool *, void *, size_t n, size_t s); void pool_free (struct pool *, void *); /* Gizmo allocations. */ diff --git a/src/tab.c b/src/tab.c index 47fd0b92..4aa7f069 100644 --- a/src/tab.c +++ b/src/tab.c @@ -47,7 +47,8 @@ struct som_table_class tab_table_class; struct tab_table * tab_create (int nc, int nr, int reallocable) { - void *(*alloc_func) (struct pool *, size_t); + void *(*alloc_func) (struct pool *, size_t n); + void *(*nalloc_func) (struct pool *, size_t n, size_t s); struct tab_table *t; @@ -65,28 +66,29 @@ tab_create (int nc, int nr, int reallocable) t->nc = t->cf = nc; t->l = t->r = t->t = t->b = 0; + nalloc_func = reallocable ? pool_nmalloc : pool_nalloc; alloc_func = reallocable ? pool_malloc : pool_alloc; #if GLOBAL_DEBUGGING t->reallocable = reallocable; #endif - t->cc = alloc_func (t->container, nr * nc * sizeof *t->cc); + t->cc = nalloc_func (t->container, nr * nc, sizeof *t->cc); t->ct = alloc_func (t->container, nr * nc); memset (t->ct, TAB_EMPTY, nc * nr); - t->rh = alloc_func (t->container, nc * (nr + 1)); + t->rh = nalloc_func (t->container, nc, nr + 1); memset (t->rh, 0, nc * (nr + 1)); - t->hrh = alloc_func (t->container, sizeof *t->hrh * (nr + 1)); + t->hrh = nalloc_func (t->container, nr + 1, sizeof *t->hrh); memset (t->hrh, 0, sizeof *t->hrh * (nr + 1)); t->trh = alloc_func (t->container, nr + 1); memset (t->trh, 0, nr + 1); - t->rv = alloc_func (t->container, (nc + 1) * nr); + t->rv = nalloc_func (t->container, nr, nc + 1); memset (t->rv, 0, (nc + 1) * nr); - t->wrv = alloc_func (t->container, sizeof *t->wrv * (nc + 1)); + t->wrv = nalloc_func (t->container, nc + 1, sizeof *t->wrv); memset (t->wrv, 0, sizeof *t->wrv * (nc + 1)); t->trv = alloc_func (t->container, nc + 1); @@ -163,7 +165,7 @@ tab_realloc (struct tab_table *t, int nc, int nr) unsigned char *new_ct; int r; - new_cc = pool_malloc (t->container, nr * nc * sizeof *new_cc); + new_cc = pool_nmalloc (t->container, nr * nc, sizeof *new_cc); new_ct = pool_malloc (t->container, nr * nc); for (r = 0; r < mr1; r++) { @@ -179,14 +181,13 @@ tab_realloc (struct tab_table *t, int nc, int nr) } else if (nr != t->nr) { - t->cc = pool_realloc (t->container, t->cc, nr * nc * sizeof *t->cc); + t->cc = pool_nrealloc (t->container, t->cc, nr * nc, sizeof *t->cc); t->ct = pool_realloc (t->container, t->ct, nr * nc); - t->rh = pool_realloc (t->container, t->rh, nc * (nr + 1)); - t->rv = pool_realloc (t->container, t->rv, (nc + 1) * nr); + t->rh = pool_nrealloc (t->container, t->rh, nc, nr + 1); + t->rv = pool_nrealloc (t->container, t->rv, nr, nc + 1); t->trh = pool_realloc (t->container, t->trh, nr + 1); - t->hrh = pool_realloc (t->container, t->hrh, - sizeof *t->hrh * (nr + 1)); + t->hrh = pool_nrealloc (t->container, t->hrh, nr + 1, sizeof *t->hrh); if (nr > t->nr) { @@ -938,8 +939,8 @@ tabi_table (struct som_entity *table) tab_offset (t, 0, 0); assert (t->w == NULL && t->h == NULL); - t->w = pool_alloc (t->container, sizeof *t->w * t->nc); - t->h = pool_alloc (t->container, sizeof *t->h * t->nr); + t->w = pool_nalloc (t->container, t->nc, sizeof *t->w); + t->h = pool_nalloc (t->container, t->nr, sizeof *t->h); } /* Set the current output device to DRIVER. */ -- 2.30.2