[Bug-gnulib] xalloc patch to check for address arithmetic overflow

[Paul Eggert]

I installed the following patch.  It follows up on our earlier
conversations about modifying xalloc so that its primitives can catch
address arithmetic overflows.  About 12 hours ago Georgi Guninski
reported a bug in GNU 'ls' that apparently is part of a
denial-of-service attack in wu-ftpd, involving address arithmetic
overflow in GNU 'ls'; this raised my priority of getting this patch
out the door.

This patch takes a conservative approach and does not remove the
XMALLOC etc. macros, but I'd like to remove them soon, after people
have had time to adjust to the new interface.

I'll submit a corresponding patch to bug-coreutils shortly for 'ls'.

Index: ChangeLog
===================================================================
RCS file: /cvsroot/gnulib/gnulib/ChangeLog,v
retrieving revision 1.107
diff -p -u -r1.107 ChangeLog
--- ChangeLog   10 Oct 2003 19:41:12 -0000      1.107
+++ ChangeLog   13 Oct 2003 06:03:52 -0000
@@ -1,3 +1,8 @@
+2003-10-12  Paul Eggert  <address@hidden>
+
+       * modules/xalloc: Do not depend on 'exit'.  Depend on 'stdbool'.
+       Change maintainer from Bruno Haible to 'all'.
+
 2003-10-10  Simon Josefsson  <address@hidden>
 
        * modules/argp (Depends-on): Add restrict and strcase.
Index: lib/ChangeLog
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/ChangeLog,v
retrieving revision 1.582
diff -p -u -r1.582 ChangeLog
--- lib/ChangeLog       8 Oct 2003 20:34:55 -0000       1.582
+++ lib/ChangeLog       13 Oct 2003 06:03:54 -0000
@@ -1,3 +1,18 @@
+2003-10-12  Paul Eggert  <address@hidden>
+
+       * lib/xalloc.h (xnmalloc, xzalloc, xnrealloc, xclone): New decls.
+       (XMALLOC, XCALLOC, XREALLOC, XFREE, CCLONE, CLONE): Deprecate,
+       and define in terms of the other primitives.
+       * lib/xmalloc.c: Include stdbool.h; do not include exit.h.
+       (SIZE_MAX): Define if not already defined.
+       (array_size_overflow): New function.
+       (xalloc_die): Abort instead of exiting if 'error' returns.
+       (xnmalloc, xnrealloc, xzalloc, xclone): New functions.
+       (xmalloc, xrealloc): Use them.
+       (xcalloc): Check for address arithmetic overflow.
+       * lib/xstrdup.c (xstrdup): Use xclone, since memcpy should be
+       a bit faster than strcpy.
+
 2003-10-08  Paul Eggert  <address@hidden>
 
        Merge getpass from libc, plus a few fixes.
Index: lib/xalloc.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xalloc.h,v
retrieving revision 1.16
diff -p -u -r1.16 xalloc.h
--- lib/xalloc.h        22 Jul 2003 22:10:56 -0000      1.16
+++ lib/xalloc.h        13 Oct 2003 06:03:54 -0000
@@ -48,31 +48,23 @@ extern char const xalloc_msg_memory_exha
    memory allocation failure.  */
 extern void xalloc_die (void) ATTRIBUTE_NORETURN;
 
-void *xmalloc (size_t n);
+void *xmalloc (size_t s);
+void *xnmalloc (size_t n, size_t s);
+void *xzalloc (size_t s);
 void *xcalloc (size_t n, size_t s);
-void *xrealloc (void *p, size_t n);
+void *xrealloc (void *p, size_t s);
+void *xnrealloc (void *p, size_t n, size_t s);
+void *xclone (void const *p, size_t s);
 char *xstrdup (const char *str);
 
-# define XMALLOC(Type, N_items) xmalloc (sizeof (Type) * (N_items))
-# define XCALLOC(Type, N_items) xcalloc (sizeof (Type), N_items)
-# define XREALLOC(Ptr, Type, N_items) xrealloc (Ptr, sizeof (Type) * (N_items))
-
-/* Declare and alloc memory for VAR of type TYPE. */
-# define NEW(Type, Var)  Type *(Var) = XMALLOC (Type, 1)
-
-/* Free VAR only if non NULL. */
-# define XFREE(Var)    \
-   do {                 \
-      if (Var)          \
-        free (Var);     \
-   } while (0)
-
-/* Return a pointer to a malloc'ed copy of the array SRC of NUM elements. */
-# define CCLONE(Src, Num) \
-  (memcpy (xmalloc (sizeof *(Src) * (Num)), Src, sizeof *(Src) * (Num)))
-
-/* Return a malloc'ed copy of SRC. */
-# define CLONE(Src) CCLONE (Src, 1)
-
+/* These macros are deprecated; they will go away soon, and are retained
+   temporarily only to ease conversion to the functions described above.  */
+# define CCLONE(p, n) xclone (p, (n) * sizeof *(p))
+# define CLONE(p) xclone (p, sizeof *(p))
+# define NEW(type, var) type *var = xmalloc (sizeof (type))
+# define XCALLOC(type, n) xcalloc (n, sizeof (type))
+# define XMALLOC(type, n) xnmalloc (n, sizeof (type))
+# define XREALLOC(p, type, n) xnrealloc (p, n, sizeof (type))
+# define XFREE(p) free (p)
 
 #endif /* !XALLOC_H_ */
Index: lib/xmalloc.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xmalloc.c,v
retrieving revision 1.27
diff -p -u -r1.27 xmalloc.c
--- lib/xmalloc.c       12 Sep 2003 20:14:10 -0000      1.27
+++ lib/xmalloc.c       13 Oct 2003 06:03:54 -0000
@@ -23,6 +23,7 @@
 
 #include "xalloc.h"
 
+#include <stdbool.h>
 #include <stdlib.h>
 
 #include "gettext.h"
@@ -30,10 +31,11 @@
 #define N_(msgid) msgid
 
 #include "error.h"
-#include "exit.h"
 #include "exitfail.h"
 
-/* The following tests require AC_PREREQ(2.54).  */
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
 
 #ifndef HAVE_MALLOC
 "you must run the autoconf test for a GNU libc compatible malloc"
@@ -46,6 +48,15 @@
 /* If non NULL, call this function when memory is exhausted. */
 void (*xalloc_fail_func) (void) = 0;
 
+/* Return true if array of N objects, each of size S, cannot exist due
+   to arithmetic overflow.  S must be nonzero.  */
+
+static inline bool
+array_size_overflow (size_t n, size_t s)
+{
+  return SIZE_MAX / s < n;
+}
+
 /* If XALLOC_FAIL_FUNC is NULL, or does return, display this message
    before exiting when memory is exhausted.  Goes through gettext. */
 char const xalloc_msg_memory_exhausted[] = N_("memory exhausted");
@@ -58,8 +69,20 @@ xalloc_die (void)
   error (exit_failure, 0, "%s", _(xalloc_msg_memory_exhausted));
   /* The `noreturn' cannot be given to error, since it may return if
      its first argument is 0.  To help compilers understand the
-     xalloc_die does terminate, call exit. */
-  exit (EXIT_FAILURE);
+     xalloc_die does terminate, call abort.  */
+  abort ();
+}
+
+/* Allocate an array of N objects, each with S bytes of memory,
+   dynamically, with error checking.  S must be nonzero.  */
+
+inline void *
+xnmalloc (size_t n, size_t s)
+{
+  void *p;
+  if (array_size_overflow (n, s) || ! (p = malloc (n * s)))
+    xalloc_die ();
+  return p;
 }
 
 /* Allocate N bytes of memory dynamically, with error checking.  */
@@ -67,10 +90,16 @@ xalloc_die (void)
 void *
 xmalloc (size_t n)
 {
-  void *p;
+  return xnmalloc (n, 1);
+}
+
+/* Change the size of an allocated block of memory P to an array of N
+   objects each of S bytes, with error checking.  S must be nonzero.  */
 
-  p = malloc (n);
-  if (p == 0)
+inline void *
+xnrealloc (void *p, size_t n, size_t s)
+{
+  if (array_size_overflow (n, s) || ! (p = realloc (p, n * s)))
     xalloc_die ();
   return p;
 }
@@ -81,21 +110,39 @@ xmalloc (size_t n)
 void *
 xrealloc (void *p, size_t n)
 {
-  p = realloc (p, n);
-  if (p == 0)
-    xalloc_die ();
-  return p;
+  return xnrealloc (p, n, 1);
+}
+
+/* Allocate S bytes of zeroed memory dynamically, with error checking.
+   There's no need for xnzalloc (N, S), since it would be equivalent
+   to xcalloc (N, S).  */
+
+void *
+xzalloc (size_t s)
+{
+  return memset (xmalloc (s), 0, s);
 }
 
-/* Allocate memory for N elements of S bytes, with error checking.  */
+/* Allocate zeroed memory for N elements of S bytes, with error
+   checking.  S must be nonzero.  */
 
 void *
 xcalloc (size_t n, size_t s)
 {
   void *p;
-
-  p = calloc (n, s);
-  if (p == 0)
+  /* Test for overflow, since some calloc implementations don't have
+     proper overflow checks.  */
+  if (array_size_overflow (n, s) || ! (p = calloc (n, s)))
     xalloc_die ();
   return p;
+}
+
+/* Clone an object P of size S, with error checking.  There's no need
+   for xnclone (P, N, S), since xclone (P, N * S) works without any
+   need for an arithmetic overflow check.  */
+
+void *
+xclone (void const *p, size_t s)
+{
+  return memcpy (xmalloc (s), p, s);
 }
Index: lib/xstrdup.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/xstrdup.c,v
retrieving revision 1.9
diff -p -u -r1.9 xstrdup.c
--- lib/xstrdup.c       19 Aug 2003 11:32:28 -0000      1.9
+++ lib/xstrdup.c       13 Oct 2003 06:03:54 -0000
@@ -29,5 +29,5 @@
 char *
 xstrdup (const char *string)
 {
-  return strcpy (xmalloc (strlen (string) + 1), string);
+  return xclone (string, strlen (string) + 1);
 }
Index: m4/ChangeLog
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/ChangeLog,v
retrieving revision 1.506
diff -p -u -r1.506 ChangeLog
--- m4/ChangeLog        10 Oct 2003 19:41:12 -0000      1.506
+++ m4/ChangeLog        13 Oct 2003 06:03:54 -0000
@@ -1,3 +1,7 @@
+2003-10-12  Paul Eggert  <address@hidden>
+
+       * xalloc.m4 (gl_PREREQ_XMALLOC): Require AC_C_INLINE.
+
 2003-10-10  Simon Josefsson  <address@hidden>
 
        * argp.m4: Add AC_C_INLINE.
Index: m4/xalloc.m4
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/xalloc.m4,v
retrieving revision 1.3
diff -p -u -r1.3 xalloc.m4
--- m4/xalloc.m4        12 Sep 2003 18:24:51 -0000      1.3
+++ m4/xalloc.m4        13 Oct 2003 06:03:54 -0000
@@ -1,4 +1,4 @@
-# xalloc.m4 serial 3
+# xalloc.m4 serial 4
 dnl Copyright (C) 2002-2003 Free Software Foundation, Inc.
 dnl This file is free software, distributed under the terms of the GNU
 dnl General Public License.  As a special exception to the GNU General
@@ -14,8 +14,10 @@ AC_DEFUN([gl_XALLOC],
 
 # Prerequisites of lib/xmalloc.c.
 AC_DEFUN([gl_PREREQ_XMALLOC], [
+  AC_REQUIRE([AC_C_INLINE])
   AC_REQUIRE([jm_FUNC_MALLOC])
   AC_REQUIRE([jm_FUNC_REALLOC])
+  :
 ])
 
 # Prerequisites of lib/xstrdup.c.
Index: modules/xalloc
===================================================================
RCS file: /cvsroot/gnulib/gnulib/modules/xalloc,v
retrieving revision 1.6
diff -p -u -r1.6 xalloc
--- modules/xalloc      15 Sep 2003 20:40:48 -0000      1.6
+++ modules/xalloc      13 Oct 2003 06:03:54 -0000
@@ -13,7 +13,7 @@ realloc
 error
 gettext
 exitfail
-exit
+stdbool
 
 configure.ac:
 gl_XALLOC
@@ -25,5 +25,5 @@ Include:
 "xalloc.h"
 
 Maintainer:
-Bruno Haible
+all