Re: [PATCH] inttostr.h: add compile-time buffer overrun checks

[Bruno Haible]

Jim Meyering wrote:
> Nice and thorough work.  Thanks for covering almost all of the
> remaining cases.

OK, here's a proposed patch, for inttostr. If that works fine, I'll continue
with the *sprintf variants in libunistring.


2010-10-17  Jim Meyering  <address@hidden>
            Bruno Haible  <address@hidden>

        Add bounds checking to the inttostr() and similar functions.
        * lib/anytostr.c (_GL_NO_FORTIFY): Define.
        Include <stdlib.h>.
        (anytostr_chk): New function.
        * lib/inttostr.h (_GL_STRINGIFY, _GL_CONCAT): New macros.
        (imaxtostr_chk, inttostr_chk, offtostr_chk, uinttostr_chk,
        umaxtostr_chk): New declarations.
        (_GL_pointed_object_size, _GL_ASM_SYMBOL_PREFIX): New macros.
        (_GL_DEFINE_INTTOSTR_FUNCS): New macro.
        (imaxtostr, inttostr, offtostr, uinttostr, umaxtostr): Redefine as
        macros with bounds checking.
        * lib/verify.h (_GL_CONCAT): Ensure no redefinition.
        * m4/inttostr.m4 (gl_PREREQ_INTTOSTR): Require gl_ASM_SYMBOL_PREFIX.
        * modules/inttostr (Files): Add m4/asm-underscore.m4.
        * modules/inttostr-tests (Files): Add tests/test-inttostr2.c.
        * tests/test-inttostr2.c: New file.

--- lib/anytostr.c.orig Sun Oct 17 21:54:43 2010
+++ lib/anytostr.c      Sun Oct 17 21:44:40 2010
@@ -19,7 +19,12 @@
 
 #include <config.h>
 
+/* Don't define inttostr etc. as macros in this compilation unit.  */
+#define _GL_NO_FORTIFY 1
+
 #include "inttostr.h"
+
+#include <stdlib.h>
 #include "verify.h"
 
 /* Convert I to a printable string in BUF, which must be at least
@@ -52,3 +57,16 @@
 
   return p;
 }
+
+/* Like anytostr.
+   Also verify that SIZE is >= INT_BUFSIZE_BOUND (INTTYPE).
+   SIZE is the number of bytes available at BUF, as determined by the
+   compiler.  */
+char * __attribute_warn_unused_result__
+_GL_CONCAT (anytostr, _chk) (inttype i, char *buf, size_t size)
+{
+  if (!(size >= INT_BUFSIZE_BOUND (inttype)))
+    /* The compiler determined that the buf argument is too small.  */
+    abort ();
+  return anytostr (i, buf);
+}
--- lib/inttostr.h.orig Sun Oct 17 21:54:43 2010
+++ lib/inttostr.h      Sun Oct 17 21:54:39 2010
@@ -39,8 +39,111 @@
 # define __attribute_warn_unused_result__ /* empty */
 #endif
 
+#ifndef _GL_STRINGIFY
+/* Convert a token to a string.  */
+# define _GL_STRINGIFY(s) _GL_STRINGIFY1 (s)
+# define _GL_STRINGIFY1(s) #s
+#endif
+
+#ifndef _GL_CONCAT
+/* Concatenate two preprocessor tokens.  */
+# define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y)
+# define _GL_CONCAT0(x, y) x##y
+#endif
+
 char *imaxtostr (intmax_t, char *) __attribute_warn_unused_result__;
+char *imaxtostr_chk (intmax_t, char *, size_t) 
__attribute_warn_unused_result__;
+
 char *inttostr (int, char *) __attribute_warn_unused_result__;
+char *inttostr_chk (int, char *, size_t) __attribute_warn_unused_result__;
+
 char *offtostr (off_t, char *) __attribute_warn_unused_result__;
+char *offtostr_chk (off_t, char *, size_t) __attribute_warn_unused_result__;
+
 char *uinttostr (unsigned int, char *) __attribute_warn_unused_result__;
+char *uinttostr_chk (unsigned int, char *, size_t) 
__attribute_warn_unused_result__;
+
 char *umaxtostr (uintmax_t, char *) __attribute_warn_unused_result__;
+char *umaxtostr_chk (uintmax_t, char *, size_t) 
__attribute_warn_unused_result__;
+
+/* When, on glibc systems, -D_FORTIFY_SOURCE=1 or -D_FORTIFY_SOURCE=2 is used,
+   enable extra bounds checking, based on the object bounds analysis done by
+   GCC.
+   The user can disable this bounds checking by defining _GL_NO_FORTIFY.
+   __attribute__ __warning__ requires GCC >= 4.3.
+   __builtin_object_size requires GCC >= 4.1.
+   __always_inline__ requires GCC >= 3.2.  */
+#if __USE_FORTIFY_LEVEL > 0 && !defined _GL_NO_FORTIFY && __GNUC_PREREQ (4, 3)
+
+/* Compiler determined size of a char* pointer. or char[] array.
+   Works via a GCC built-in for pointers into arrays of constant size,
+   and works via sizeof() for references to arrays of variable length.
+   Punt for pointers to arrays of variable length or unknown length.  */
+# define _GL_pointed_object_size(s) \
+  (__builtin_object_size (s, 1) != (size_t)-1 \
+   ? __builtin_object_size (s, 1)             \
+   : sizeof (s) != sizeof (void *)            \
+     ? sizeof (s)                             \
+     : (size_t)-1)
+
+/* The prefix of C symbols at the assembly language level and the linker level,
+   as a string.  USER_LABEL_PREFIX is determined by gl_ASM_SYMBOL_PREFIX.  */
+# define _GL_ASM_SYMBOL_PREFIX _GL_STRINGIFY (USER_LABEL_PREFIX)
+
+/* Define auxiliary functions for bounds checking of FUNC.
+   The FUNC_chk_warn function behaves like the FUNC_chk function but emits a
+   GCC compile-time warning attached to it.  It has to be an 'extern' function,
+   not inline, due to the way GCC's __warning__ attribute works.
+   The FUNC_chk_inline function exists so that the arguments of FUNC are
+   evaluated only once, that is, so that side effects in the arguments happen
+   exactly once.  */
+# define _GL_DEFINE_INTTOSTR_FUNCS(FUNC,TYPE) \
+  extern char *                                                               \
+  __attribute__((                                                             \
+    __warning__ (_GL_STRINGIFY(FUNC) ": size of destination buffer too small")\
+  ))                                                                          \
+  _GL_CONCAT(FUNC,_chk_warn) (TYPE n, char *s, size_t size)                   \
+  __asm__ (_GL_ASM_SYMBOL_PREFIX _GL_STRINGIFY(FUNC) "_chk");                 \
+                                                                              \
+  static inline __attribute__ ((__always_inline__)) char *                    \
+  _GL_CONCAT(FUNC,_chk_inline) (TYPE n, char *s, size_t size)                 \
+  {                                                                           \
+    if (__builtin_constant_p (size))                                          \
+      {                                                                       \
+        if (size != (size_t)-1)                                               \
+          {                                                                   \
+            /* buffer size known at compile time.  */                         \
+            if (size >= INT_BUFSIZE_BOUND (TYPE))                             \
+              return (FUNC) (n, s);                                           \
+            else                                                              \
+              return _GL_CONCAT(FUNC,_chk_warn) (n, s, size);                 \
+          }                                                                   \
+        else                                                                  \
+          /* buffer size unknown.  */                                         \
+          return (FUNC) (n, s);                                               \
+      }                                                                       \
+    /* buffer size unknown at compile time but known at run time.  */         \
+    return _GL_CONCAT(FUNC,_chk) (n, s, size);                                \
+  }
+
+_GL_DEFINE_INTTOSTR_FUNCS (imaxtostr, intmax_t)
+# define imaxtostr(n, s) \
+  (__extension__ (imaxtostr_chk_inline (n, s, _GL_pointed_object_size (s))))
+
+_GL_DEFINE_INTTOSTR_FUNCS (inttostr, int)
+# define inttostr(n, s) \
+  (__extension__ (inttostr_chk_inline (n, s, _GL_pointed_object_size (s))))
+
+_GL_DEFINE_INTTOSTR_FUNCS (offtostr, off_t)
+# define offtostr(n, s) \
+  (__extension__ (offtostr_chk_inline (n, s, _GL_pointed_object_size (s))))
+
+_GL_DEFINE_INTTOSTR_FUNCS (uinttostr, unsigned int)
+# define uinttostr(n, s) \
+  (__extension__ (uinttostr_chk_inline (n, s, _GL_pointed_object_size (s))))
+
+_GL_DEFINE_INTTOSTR_FUNCS (umaxtostr, uintmax_t)
+# define umaxtostr(n, s) \
+  (__extension__ (umaxtostr_chk_inline (n, s, _GL_pointed_object_size (s))))
+
+#endif
--- lib/verify.h.orig   Sun Oct 17 21:54:43 2010
+++ lib/verify.h        Sun Oct 17 21:33:55 2010
@@ -122,9 +122,11 @@
    * In C++, any struct definition inside sizeof is invalid.
      Use a template type to work around the problem.  */
 
+# ifndef _GL_CONCAT
 /* Concatenate two preprocessor tokens.  */
-# define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y)
-# define _GL_CONCAT0(x, y) x##y
+#  define _GL_CONCAT(x, y) _GL_CONCAT0 (x, y)
+#  define _GL_CONCAT0(x, y) x##y
+# endif
 
 /* _GL_COUNTER is an integer, preferably one that changes each time we
    use it.  Use __COUNTER__ if it works, falling back on __LINE__
--- m4/inttostr.m4.orig Sun Oct 17 21:54:43 2010
+++ m4/inttostr.m4      Sun Oct 17 20:53:23 2010
@@ -1,4 +1,4 @@
-#serial 8
+#serial 9
 dnl Copyright (C) 2004, 2005, 2006, 2009, 2010 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -16,6 +16,7 @@
 # Prerequisites of lib/inttostr.h.
 AC_DEFUN([gl_PREREQ_INTTOSTR], [
   AC_REQUIRE([AC_TYPE_OFF_T])
+  AC_REQUIRE([gl_ASM_SYMBOL_PREFIX])
   :
 ])
 
--- modules/inttostr.orig       Sun Oct 17 21:54:43 2010
+++ modules/inttostr    Sun Oct 17 20:52:40 2010
@@ -10,6 +10,7 @@
 lib/umaxtostr.c
 lib/uinttostr.c
 m4/inttostr.m4
+m4/asm-underscore.m4
 
 Depends-on:
 intprops
--- modules/inttostr-tests.orig Sun Oct 17 21:54:43 2010
+++ modules/inttostr-tests      Sun Oct 17 21:29:19 2010
@@ -1,6 +1,7 @@
 Files:
 tests/macros.h
 tests/test-inttostr.c
+tests/test-inttostr2.c
 
 Depends-on:
 intprops
=========================== tests/test-inttostr2.c ===========================
/* Test Fortify support for inttostr functions.
   Copyright (C) 2010 Free Software Foundation, Inc.

   This program is free software: you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 3 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

/* Compile this with a GCC >= 4.3 on a glibc system, with options -O -S.  */

/* Defining _FORTIFY_SOURCE, together with option -O, on a glibc system
   leads to a definition of __USE_FORTIFY_LEVEL.  */
#undef _FORTIFY_SOURCE
#define _FORTIFY_SOURCE 1

#include <config.h>

#include "inttostr.h"

#include <string.h>

/* Buffer too small: warning at compile time, and call inttostr_chk.  */

char *
test_small_1 (int n)
{
  char buf[5];
  return strdup (inttostr (n, buf)); /* expect warning here */
}

char *
test_small_2 (int n)
{
  char buf[5];
  return strdup (inttostr (n, &buf[0])); /* expect warning here */
}

/* Buffer large enough: no warning, and call inttostr.  */

char *
test_large_1 (int n)
{
  char buf[12];
  return strdup (inttostr (n, buf));
}

char *
test_large_2 (int n)
{
  char buf[12];
  return strdup (inttostr (n, &buf[0]));
}

/* Buffer size unknown at compile time but known at run time.
   No warning, but call inttostr_chk.  */

char *
test_variable_length_array (int n)
{
  char buf[n < 0 ? 21 : 20];
  return strdup (inttostr (n, buf));
}

/* Buffer size unknown.
   No warning. Call inttostr.  */

char *
test_variable_length_array_pointer (int n)
{
  char buf[n < 0 ? 21 : 20];
  return strdup (inttostr (n, &buf[0]));
}

/*
 * For Emacs M-x compile
 * Local Variables:
 * compile-command: "gcc -I. -I.. -I../gllib  -O -S test-inttostr2.c"
 * End:
 */