Module Name: src Committed By: christos Date: Thu Feb 5 16:04:35 UTC 2015
Modified Files: src/lib/libc/regex: regcomp.c src/lib/libc/stdlib: Makefile.inc malloc.3 Added Files: src/lib/libc/stdlib: reallocarray.c Log Message: Add and use reallocarray() to prevent a multiplication overflow in allocation. Reported by Guido Vranken, thanks! To generate a diff of this commit: cvs rdiff -u -r1.33 -r1.34 src/lib/libc/regex/regcomp.c cvs rdiff -u -r1.86 -r1.87 src/lib/libc/stdlib/Makefile.inc cvs rdiff -u -r1.38 -r1.39 src/lib/libc/stdlib/malloc.3 cvs rdiff -u -r0 -r1.1 src/lib/libc/stdlib/reallocarray.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/lib/libc/regex/regcomp.c diff -u src/lib/libc/regex/regcomp.c:1.33 src/lib/libc/regex/regcomp.c:1.34 --- src/lib/libc/regex/regcomp.c:1.33 Tue Mar 13 17:13:43 2012 +++ src/lib/libc/regex/regcomp.c Thu Feb 5 11:04:35 2015 @@ -1,4 +1,4 @@ -/* $NetBSD: regcomp.c,v 1.33 2012/03/13 21:13:43 christos Exp $ */ +/* $NetBSD: regcomp.c,v 1.34 2015/02/05 16:04:35 christos Exp $ */ /*- * Copyright (c) 1992, 1993, 1994 @@ -76,11 +76,12 @@ #if 0 static char sccsid[] = "@(#)regcomp.c 8.5 (Berkeley) 3/20/94"; #else -__RCSID("$NetBSD: regcomp.c,v 1.33 2012/03/13 21:13:43 christos Exp $"); +__RCSID("$NetBSD: regcomp.c,v 1.34 2015/02/05 16:04:35 christos Exp $"); #endif #endif /* LIBC_SCCS and not lint */ #include "namespace.h" +#include <sys/param.h> #include <sys/types.h> #include <assert.h> @@ -262,12 +263,11 @@ regcomp( len = strlen(pattern); /* do the mallocs early so failure handling is easy */ - g = (struct re_guts *)malloc(sizeof(struct re_guts) + - (NC-1)*sizeof(cat_t)); + g = malloc(sizeof(struct re_guts) + (NC - 1) * sizeof(cat_t)); if (g == NULL) return(REG_ESPACE); p->ssize = len/(size_t)2*(size_t)3 + (size_t)1; /* ugh */ - p->strip = malloc(p->ssize * sizeof(sop)); + p->strip = reallocarray(NULL, p->ssize, sizeof(sop)); p->slen = 0; if (p->strip == NULL) { free(g); @@ -1249,14 +1249,15 @@ allocset( if (MEMSIZE(p) > MEMLIMIT) goto oomem; if (p->g->sets == NULL) - p->g->sets = malloc(nc * sizeof(cset)); + p->g->sets = reallocarray(NULL, nc, sizeof(cset)); else - p->g->sets = realloc(p->g->sets, nc * sizeof(cset)); + p->g->sets = reallocarray(p->g->sets, nc, sizeof(cset)); if (p->g->setbits == NULL) p->g->setbits = malloc(nbytes); else { p->g->setbits = realloc(p->g->setbits, nbytes); - /* xxx this isn't right if setbits is now NULL */ + if (p->g->setbits == NULL) + goto oomem; for (i = 0; i < no; i++) p->g->sets[i].ptr = p->g->setbits + css*(i/CHAR_BIT); } @@ -1779,7 +1780,7 @@ enlarge( p->ssize = size; if (MEMSIZE(p) > MEMLIMIT) goto oomem; - sp = realloc(p->strip, p->ssize * sizeof(sop)); + sp = reallocarray(p->strip, p->ssize, sizeof(sop)); if (sp == NULL) { oomem: p->ssize = osize; @@ -1804,7 +1805,7 @@ stripsnug( _DIAGASSERT(g != NULL); g->nstates = p->slen; - g->strip = realloc(p->strip, p->slen * sizeof(sop)); + g->strip = reallocarray(p->strip, p->slen, sizeof(sop)); if (g->strip == NULL) { SETERROR(REG_ESPACE); g->strip = p->strip; Index: src/lib/libc/stdlib/Makefile.inc diff -u src/lib/libc/stdlib/Makefile.inc:1.86 src/lib/libc/stdlib/Makefile.inc:1.87 --- src/lib/libc/stdlib/Makefile.inc:1.86 Sun Jan 18 12:59:36 2015 +++ src/lib/libc/stdlib/Makefile.inc Thu Feb 5 11:04:35 2015 @@ -1,4 +1,4 @@ -# $NetBSD: Makefile.inc,v 1.86 2015/01/18 17:59:36 christos Exp $ +# $NetBSD: Makefile.inc,v 1.87 2015/02/05 16:04:35 christos Exp $ # from: @(#)Makefile.inc 8.3 (Berkeley) 2/4/95 # stdlib sources @@ -12,7 +12,7 @@ SRCS+= _env.c _rand48.c \ lcong48.c lrand48.c lsearch.c merge.c mi_vector_hash.c mrand48.c \ nrand48.c putenv.c qabs.c qdiv.c qsort.c posix_openpt.c pty.c \ quick_exit.c radixsort.c rand.c rand_r.c random.c remque.c \ - seed48.c setenv.c srand48.c strsuftoll.c \ + reallocarray.c seed48.c setenv.c srand48.c strsuftoll.c \ strtoi.c strtou.c strtonum.c \ strtoimax.c strtol.c strtoll.c strtoq.c strtoul.c strtoull.c \ strtoumax.c strtouq.c system.c tdelete.c tfind.c tsearch.c twalk.c \ @@ -74,6 +74,7 @@ MLINKS+=hcreate.3 hdestroy1.3 hcreate.3 MLINKS+=insque.3 remque.3 MLINKS+=lsearch.3 lfind.3 MLINKS+=malloc.3 calloc.3 malloc.3 realloc.3 malloc.3 free.3 +MLINKS+=malloc.3 reallocarray.3 MLINKS+=qsort.3 heapsort.3 qsort.3 mergesort.3 MLINKS+=ptsname.3 ptsname_r.3 MLINKS+=rand.3 rand_r.3 Index: src/lib/libc/stdlib/malloc.3 diff -u src/lib/libc/stdlib/malloc.3:1.38 src/lib/libc/stdlib/malloc.3:1.39 --- src/lib/libc/stdlib/malloc.3:1.38 Mon May 3 04:23:20 2010 +++ src/lib/libc/stdlib/malloc.3 Thu Feb 5 11:04:35 2015 @@ -1,4 +1,4 @@ -.\" $NetBSD: malloc.3,v 1.38 2010/05/03 08:23:20 jruoho Exp $ +.\" $NetBSD: malloc.3,v 1.39 2015/02/05 16:04:35 christos Exp $ .\" .\" Copyright (c) 1980, 1991, 1993 .\" The Regents of the University of California. All rights reserved. @@ -34,11 +34,11 @@ .\" @(#)malloc.3 8.1 (Berkeley) 6/4/93 .\" $FreeBSD: src/lib/libc/stdlib/malloc.3,v 1.73 2007/06/15 22:32:33 jasone Exp $ .\" -.Dd May 3, 2010 +.Dd February 5, 2015 .Dt MALLOC 3 .Os .Sh NAME -.Nm malloc , calloc , realloc , free +.Nm malloc , calloc , realloc , reallocarray, free .Nd general purpose memory allocation functions .Sh LIBRARY .Lb libc @@ -50,6 +50,8 @@ .Fn calloc "size_t number" "size_t size" .Ft void * .Fn realloc "void *ptr" "size_t size" +.Ft void * +.Fn reallocarray "void *ptr" "size_t number" "size_t size" .Ft void .Fn free "void *ptr" .Sh DESCRIPTION @@ -74,7 +76,7 @@ The result is identical to calling with an argument of .Dq "number * size" , with the exception that the allocated memory is explicitly initialized -to zero bytes. +to zero bytes, and overflow is being checked. .Pp The .Fn realloc @@ -90,8 +92,22 @@ the value of the newly allocated portion Upon success, the memory referenced by .Fa ptr is freed and a pointer to the newly allocated memory is returned. +.Pp +The +.Fn reallocarray +function is similar to +.Fn realloc +except it operates on +.Fa number +members of size +.Fa size +and checks for integer overflow in the calculation of\ +.Dq "number * size" . +.Pp Note that .Fn realloc +and +.Fn reallocarray may move the memory allocation, resulting in a different return value than .Fa ptr . If @@ -129,7 +145,9 @@ is set to .Pp The .Fn realloc -function returns a pointer, possibly identical to +and +.Fn reallocarray +functions return a pointer, possibly identical to .Fa ptr , to the allocated memory if successful; otherwise a @@ -141,7 +159,9 @@ is set to if the error was the result of an allocation failure. The .Fn realloc -function always leaves the original buffer intact +and +.Fn reallocarray +functions always leave the original buffer intact when an error occurs. .Pp The @@ -158,7 +178,7 @@ if ((p = malloc(number * size)) == NULL) .Pp The multiplication may lead to an integer overflow. To avoid this, -.Fn calloc +.Fn reallocarray is recommended. .Pp If @@ -171,6 +191,42 @@ if (size && number > SIZE_MAX / size) { } .Ed .Pp +The above test is not sufficient in all cases. +For example, multiplying ints requires a different set of checks: +.Bd -literal -offset indent +int num, size; +\&.\&.\&. + +/* Avoid invalid requests */ +if (size < 0 || num < 0) + errc(1, EOVERFLOW, "overflow"); + +/* Check for signed int overflow */ +if (size && num > INT_MAX / size) + errc(1, EOVERFLOW, "overflow"); + +if ((p = malloc(size * num)) == NULL) + err(1, "malloc"); +.Ed +.Pp +Assuming the implementation checks for integer overflow as +.Nx +does, it is much easier to use +.Fn calloc +or +.Fn reallocarray . +.Pp +The above examples could be simplified to: +.Bd -literal -offset indent +if ((p = reallocarray(NULL, num, size)) == NULL) + err(1, "reallocarray"); +.Ed +.Bd -literal -offset indent +or at the cost of initialization: +if ((p = calloc(num, size)) == NULL) + err(1, "calloc"); +.Ed +.Pp When using .Fn realloc , one must be careful to avoid the following idiom: @@ -227,3 +283,10 @@ and .Fn free functions conform to .St -isoC . +.Pp +The +.Fn reallocarray +function first appeared on +.Ox 5.6 +and +.Nx 8 . Added files: Index: src/lib/libc/stdlib/reallocarray.c diff -u /dev/null src/lib/libc/stdlib/reallocarray.c:1.1 --- /dev/null Thu Feb 5 11:04:35 2015 +++ src/lib/libc/stdlib/reallocarray.c Thu Feb 5 11:04:35 2015 @@ -0,0 +1,41 @@ +/* $OpenBSD: reallocarray.c,v 1.1 2014/05/08 21:43:49 deraadt Exp $ */ +/* + * Copyright (c) 2008 Otto Moerbeek <o...@drijf.net> + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ +#include <sys/cdefs.h> +__RCSID("$NetBSD: reallocarray.c,v 1.1 2015/02/05 16:04:35 christos Exp $"); + +#include <sys/param.h> +#include <sys/types.h> +#include <errno.h> +#include <stdint.h> +#include <stdlib.h> + +/* + * This is sqrt(SIZE_MAX+1), as s1*s2 <= SIZE_MAX + * if both s1 < MUL_NO_OVERFLOW and s2 < MUL_NO_OVERFLOW + */ +#define MUL_NO_OVERFLOW ((size_t)1 << (sizeof(size_t) * (CHAR_BIT / 2))) + +void * +reallocarray(void *optr, size_t nmemb, size_t size) +{ + if ((nmemb >= MUL_NO_OVERFLOW || size >= MUL_NO_OVERFLOW) && + size > 0 && nmemb > 0 && SIZE_MAX / nmemb < size) { + errno = ENOMEM; + return NULL; + } + return realloc(optr, size * nmemb); +}