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);
+}

Reply via email to